[SERVER-36902] mongo shell does not abort transaction on quit Created: 28/Aug/18 Updated: 08/Jan/24 Resolved: 03/Jan/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Shell |
| Affects Version/s: | 4.0.1, 4.1.2 |
| Fix Version/s: | 4.1.7 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Wan Bachtiar | Assignee: | A. Jesse Jiryu Davis |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Sprint: | Repl 2018-12-17, Repl 2019-01-14 |
| Participants: |
| Description |
|
After executing below code in mongo shell:
If the user then quit (SIGINT) the shell, the transaction will still be alive in the server until transactionLifetimeLimitSeconds is reached. Snippet MongoDB log showed that there is endSessions sent, but transaction within the session is not aborted.
Until the transaction life time is reached:
|
| Comments |
| Comment by A. Jesse Jiryu Davis [ 03/Jan/19 ] | ||||||||
|
Yeah, just waiting a bit for a final patch build before I pushed. | ||||||||
| Comment by Githook User [ 03/Jan/19 ] | ||||||||
|
Author: {'username': 'ajdavis', 'email': 'jesse@mongodb.com', 'name': 'A. Jesse Jiryu Davis'}Message: | ||||||||
| Comment by Eric Milkie [ 03/Jan/19 ] | ||||||||
|
Was there a final commit for this? | ||||||||
| Comment by A. Jesse Jiryu Davis [ 20/Dec/18 ] | ||||||||
|
I thought I'd handled a rebase issue easily but I made a mistake. Fixing.... | ||||||||
| Comment by Ramon Fernandez Marina [ 20/Dec/18 ] | ||||||||
|
Reopening since one of the commits was reverted; best to not ship 4.1.7 without the test and have to make another ticket. Please re-close if this is not the right thing to do. | ||||||||
| Comment by Githook User [ 20/Dec/18 ] | ||||||||
|
Author: {'username': 'benety', 'email': 'benety@mongodb.com', 'name': 'Benety Goh'}Message: Revert " This reverts commit 759846ffced5ef84734bd917a99061edf44dd786. | ||||||||
| Comment by Githook User [ 20/Dec/18 ] | ||||||||
|
Author: {'username': 'dgottlieb', 'email': 'daniel.gottlieb@mongodb.com', 'name': 'Daniel Gottlieb'}Message: Revert " This reverts commit bf58b1ab2abfb2a3ab7a86c154f9f5954ed6f98c. | ||||||||
| Comment by Githook User [ 20/Dec/18 ] | ||||||||
|
Author: {'username': 'ajdavis', 'email': 'jesse@mongodb.com', 'name': 'A. Jesse Jiryu Davis'}Message: | ||||||||
| Comment by Githook User [ 20/Dec/18 ] | ||||||||
|
Author: {'username': 'ajdavis', 'email': 'jesse@mongodb.com', 'name': 'A. Jesse Jiryu Davis'}Message: | ||||||||
| Comment by Kevin Pulo [ 20/Sep/18 ] | ||||||||
Thanks, this is what I had missed. Since it's the responsibility of conforming clients to avoid calling endSessions when there's an active transaction, I agree that fixing the shell is appropriate (and there's no need to adjust server behaviour). | ||||||||
| Comment by A. Jesse Jiryu Davis [ 19/Sep/18 ] | ||||||||
|
SGTM. To clarify Shane's example: a client should end any session it started before the client exits, and that includes trying to abort any transaction that is in progress on any session. Shane's just pointing out that if a client is killed with no chance to clean up, then it might leave sessions open, including sessions with transactions. | ||||||||
| Comment by Tess Avitabile (Inactive) [ 19/Sep/18 ] | ||||||||
|
Thanks, shane.harvey. In that case, I think the shell should run abortTransaction before endSessions, so that it conforms more closely to the drivers spec. Based on your example, perhaps it is not always the correct behavior for the shell to run endSessions on exit, but I'm not sure this is important to change. | ||||||||
| Comment by Shane Harvey [ 19/Sep/18 ] | ||||||||
|
The transaction spec already mandates that drivers MUST abort any in progress transaction before ending a ClientSession so I think the benefits would be limited to cases where the implicit abortTransaction fails (twice). It's worth noting that leaving in progress transactions open might be the expected behavior depending on the language. For example, this code would leave a transaction open on the server similar to shell example above:
While this code would automatically abort the transaction:
Drivers make a best effort to implicitly end sessions/transactions/cursors/etc.. whenever possible but in general it's the user's responsibility to gracefully release their resources. | ||||||||
| Comment by Tess Avitabile (Inactive) [ 19/Sep/18 ] | ||||||||
|
There is probably not a downside to defining the behavior that way, since running abortTransaction is inexpensive. It is not something we considered in the transactions design. shane.harvey, jesse, would it be beneficial to drivers if endSessions additionally aborted any in-progress transaction? Or do drivers already abort any in-progress transaction prior to running endSessions upon exiting or leaving the scope of the session? | ||||||||
| Comment by Kevin Pulo [ 19/Sep/18 ] | ||||||||
|
tess.avitabile is there defined behaviour somewhere for what happens if a client runs endSessions on a session that has an active in-progress transaction? Surely this should cause the transaction to be aborted, as if the client had first run abortTranaction? Is there a downside to defining the behaviour this way? | ||||||||
| Comment by Tess Avitabile (Inactive) [ 18/Sep/18 ] | ||||||||
|
Based on discussion with max.hirschhorn and mira.carey@mongodb.com, it would be preferable for the shell to abort in-progress transactions on quit. It kills cursors on quit, so it would be consistent and useful to also abort transactions. Note that the shell uses endSessions instead of killSessions on quit because killSessions is an expensive operation in a sharded cluster that must talk to all replica set members of all shards. killSessions is really only meant to be run by an operator, unlike endSessions, which is cheap and can be run often by drivers and the shell. For this reason, when we implement this work, we should not use killSessions to abort in-progress transactions, but instead run abortTransaction in addition to endSessions. abortTransaction only needs to talk to the primary of each shard that participated in the transaction. | ||||||||
| Comment by Kevin Pulo [ 29/Aug/18 ] | ||||||||
|
So we can fix the shell to abort any in-progress transaction before ending the session, but there's not really anything stopping any client from behaving in this way, ie. sending endSession while there's an in-progress transaction. Since:
I think it might be better to instead fix the endSession command on the server side, so any in-progress transaction is aborted (and this is also reported in the returned endSession result). |