Skip to content

PHPC-1359: Prohibit startTransaction() on sharded clusters #984

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 1 commit into from
Jun 3, 2019

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented May 31, 2019

https://jira.mongodb.org/browse/PHPC-1359

In versions of the driver that do not support mongos pinning, we should prohibit starting a transaction on a sharded cluster (even if the server version might actually support transactions).

@jmikola jmikola requested a review from kvwalker May 31, 2019 15:07
@jmikola jmikola force-pushed the 1.5-phpc-1359 branch 2 times, most recently from d5b3421 to 1746b92 Compare May 31, 2019 16:52
Copy link
Contributor

@kvwalker kvwalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Do you also need to check maxWireVersion < 7?

@jmikola
Copy link
Member Author

jmikola commented May 31, 2019

Do you also need to check maxWireVersion < 7?

Great question! I spoke to @kevinAlbs about this and he said it looks like libmongoc does not implement that check. He also cited the following from the Transaction spec's startTransaction section:

startTransaction SHOULD report an error if the driver can detect that transactions are not supported by the deployment. A deployment does not support transactions when the deployment does not support sessions, or maxWireVersion < 7, or the maxWireVersion < 8 and the topology type is Sharded

Since transactions cannot be used without sessions, and libmongoc does report an error if sessions are unsupported, we need only worry about starting a transaction on a 3.6 server. In that case, the driver can rely on a 3.6 server to report an error for an unrecognized command field. Testing this locally, I can produce the following server error message attempting to start a transaction with an insert command:

BSON field 'insert.startTransaction' is an unknown field

I will open a ticket to propose that libmongoc add the check to better comply with the spec, as I don't think we have good justification for ignoring that SHOULD; however, I think the change in this PR (and CDRIVER-3067) is more independent and more important, since it addresses a scenario where a driver might successfully start a transaction on a mongos server, but then fail to pin the session and invite unexpected behavior/errors at runtime.

@jmikola
Copy link
Member Author

jmikola commented May 31, 2019

Created CDRIVER-3161 to track the libmongoc issue, and opened PHPC-1391 to depend on it; however, I don't anticipate making a PHP-specific fix for that issue.

In versions of the driver that do not support mongos pinning, we should prohibit starting a transaction on a sharded cluster (even if the server version might actually support transactions).
@jmikola jmikola merged commit ce2e43e into mongodb:v1.5 Jun 3, 2019
jmikola added a commit that referenced this pull request Jun 3, 2019
@jmikola jmikola deleted the 1.5-phpc-1359 branch June 3, 2019 15:32
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.

2 participants