Skip to content

CDRIVER-4718 store recoveryToken for Load Balanced topology #1417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 17, 2023

Conversation

kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Sep 28, 2023

Summary

  • Store recoveryToken for Load Balanced topology

Background & Motivation

Bug was discovered running proposed specification test changes with the PHP driver: mongodb/specifications#1459 (comment)

Additionally, several other pinning tests fail due to recoveryToken fields not being found in outgoing commitTransaction and abortTransaction commands

The Load Balancer specification notes:

When executing a transaction in load balancing mode, drivers MUST follow the rules outlined in Sharded Transactions

Sharded Transactions describes expected behavior for recoveryToken:

Drivers MUST track the most recently received recoveryToken field and MUST append this field to any subsequent commitTransaction or abortTransaction commands.

Aside: Connection pinning

The Load Balancer specification describes required behavior for pinning all transaction operations to one connection:

The rules for pinning to a connection and releasing a pinned connection are the same as those for server pinning in non-load balanced sharded transactions

This is intentionally not implemented in libmongoc due to not being needed. CDRIVER-4056 notes:

Not in scope
Connection pinning to cursors or transactions. Rationale: The C driver does not implement a CMAP compliant connection pool (see CDRIVER-2871). Only one connection is available to a server for all operations within a client.

This motivated keeping a separate _in_sharded_txn (to check for mongos pinning) from _in_sharded_or_loadbalanced_txn (to check for recoveryToken)

@kevinAlbs kevinAlbs marked this pull request as ready for review October 11, 2023 18:48
kevinAlbs and others added 2 commits October 12, 2023 12:39
Co-authored-by: Kyle Kloberdanz <[email protected]>
@kevinAlbs kevinAlbs merged commit b39e2eb into mongodb:master Oct 17, 2023
kevinAlbs added a commit that referenced this pull request Oct 17, 2023
* add regression test

* format mongoc-cluster.c

* store recoveryToken for loadBalanced topology

* make `_in_sharded_txn` static

* use `static`

Co-authored-by: Kyle Kloberdanz <[email protected]>

* removing redundant calls to `_mongoc_topology_get_type`

Co-authored-by: Kyle Kloberdanz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants