[SERVER-36240] Make transactions on mongos use snapshot level readConcern if no level was provided Created: 23/Jul/18  Updated: 29/Oct/23  Resolved: 01/Aug/18

Status: Closed
Project: Core Server
Component/s: Sharding
Affects Version/s: None
Fix Version/s: 4.1.2

Type: Task Priority: Major - P3
Reporter: Jack Mulrow Assignee: Jack Mulrow
Resolution: Fixed Votes: 0
Labels: ShardedTxn:RouterSupport
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Documented
is documented by DOCS-11944 Docs for SERVER-36240: Make transacti... Closed
Related
is related to SERVER-35709 mongos should remember readConcern se... Closed
Backwards Compatibility: Fully Compatible
Sprint: Sharding 2018-07-30, Sharding 2018-08-13
Participants:

 Description   

If the first statement in a transaction does not specify snapshot readConcern, mongod will implicitly upconvert it to snapshot. Mongos currently does not have this behavior, and because many core/txns tests rely on this, either mongos will need to upconvert readConcerns to snapshot as well (until we support transactions with other readConcern levels) or the tests need to be updated to explicitly choose snapshot readConcern.



 Comments   
Comment by Githook User [ 01/Aug/18 ]

Author:

{'name': 'Jack Mulrow', 'email': 'jack.mulrow@mongodb.com', 'username': 'jsmulrow'}

Message: SERVER-36240 Make transactions on mongos use snapshot level readConcern if no level was provided
Branch: master
https://github.com/mongodb/mongo/commit/be2588ebda13c512cde2d7999a0deebb2531004c

Comment by Randolph Tan [ 30/Jul/18 ]

The current session code does not know anything about the read concern, so the refactor work won't fix this.

Comment by Esha Maharishi (Inactive) [ 30/Jul/18 ]

Hmm, I see, it's literally the "check readConcern, oh hey I need to upconvert" logic that you're worried about being duplicated. The fact that mongos has to do work because of the resulting upconverted readConcern is orthogonal.

I'm hesitant to modify the tests or shell... can you work with renctan to figure out how much duplicate code this would result in, and whether the Session refactor will actually be able to address it?

Comment by Jack Mulrow [ 30/Jul/18 ]

Sorry I think my comments were a little confusing, I only meant we might throw away work to upconvert local or majority readConcern to snapshot once we support more than just snapshot on mongod. I agree we'll always want to do something on mongos when there is no readConcern.

If we want to keep the behavior of upconverting no readConcern to snapshot, then that's what I'll add to mongos as part of this ticket. And if we end up deciding to change the default level later, it shouldn't be hard to do.

Comment by Esha Maharishi (Inactive) [ 30/Jul/18 ]

My understanding is that we want to preserve the 4.0 behavior of no readConcern being upconverted to snapshot.

In order to do that in a cluster, mongos has to check if it needs to upconvert to snapshot (it can't just let the shards do this), because mongos has to choose the atClusterTime for snapshot.

We will need to modify the user's readConcern anyway to include the atClusterTime... I guess I'm a little confused why the work to upconvert to snapshot on mongos would get thrown away?

Comment by Jack Mulrow [ 27/Jul/18 ]

Since I think we're planning on supporting local and majority level transactions this release, it feels like a waste to spend time implementing it on mongos just to remove it later. Also I think writing the code to do this would be a little awkward, since we can't just change the value stored on the readConcern operation context decoration like mongod does, we'd probably also want to modify the user's request itself when we parse its readConcern, which I'd rather avoid if we don't really have to do it.

Comment by Esha Maharishi (Inactive) [ 27/Jul/18 ]

Why shouldn't we add the upconverting behavior to mongos? Is it because the code is not in a state to do that yet?

Comment by Randolph Tan [ 26/Jul/18 ]

If we make mongos return an error if readConcern is not specified, this would mean that it would behave differently with mongod. If we are gonna go that route, we probably need to make sure that the drivers would always supply readConcern when starting a transaction in mongos. I don't like the option of changing the default to snapshot in the shell but the first bullet option sounds good to me.

Comment by Jack Mulrow [ 25/Jul/18 ]

The real purpose of this ticket is just to figure out how mongos should deal with transactions that don't specify a readConcern. Once SERVER-35709 is committed, mongos will inspect requests with startTransaction for a readConcern and uassert if there isn't one (since I wasn't sure what else to have it do), so we'll have to do something if we want to run most of the core/txns suite against mongos.

If we want to delay making an actual decision, I can think of two straightforward ways to get the txns suite running:

  • Add explicit snapshot readConcerns to the tests that rely on upconverting (except the ones that specifically test upconverting). They already run with snapshot readConcern, since that's the only level mongod supports, so making it explicit won't change behavior.
  • Change the shell transaction API to use readConcern snapshot as the default if the user doesn't provide one. That way we won't need to modify any tests, but I think the behavior would be more confusing and we'd have to remember to undo the change at some point.

kaloian.manassiev, esha.maharishi, renctan what do you guys think?

Generated at Thu Feb 08 04:42:30 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.