[SERVER-47079] Investigate and remove unnecessary causalConsistency=false settings in sharding testing Created: 24/Mar/20 Updated: 06/Dec/22 Resolved: 24/Apr/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Testing Infrastructure |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Maria van Keulen | Assignee: | [DO NOT USE] Backlog - Sharding Team |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Assigned Teams: |
Sharding
|
||||||||
| Participants: | |||||||||
| Description |
|
Currently, a fair amount of our testing, especially transactions testing, uses sessions with causalConsistency explicitly set to false (as opposed to using the default). Per a discussion with judah.schvimer, this setting is left over from early transactions testing, and is not necessarily required for new tests. |
| Comments |
| Comment by Kaloian Manassiev [ 24/Apr/20 ] |
|
This ticket is just cleanup, which doesn't change the testing coverage, so we decided to not invest time in it. |
| Comment by Judah Schvimer [ 16/Apr/20 ] |
|
I agree that this is not a priority. I have preferred the convention to be to not specify {causalConsistency: false} but don't feel strongly enough to codify it or make it uniform across the codebase. |
| Comment by Max Hirschhorn [ 16/Apr/20 ] |
|
It looks like the driver examples in https://docs.mongodb.com/manual/core/transactions-in-applications/ all show calling startSession() without changing the causal consistency setting, so they'll use the default of true. I agree it is likely that'll be the more common way our users end up using transactions then. We should already be running the tests in the jstests/core/txns/ directory with causal consistency enabled (regardless of whether the file itself specifies {causalConsistency: true} or {causalConsistency: false}) in the
test suites. I believe we generally support read-your-writes behavior when all reads and writes (including ones from multi-statement transactions) happen against the primary and the primary doesn't change, so that may actually be an argument for {causalConsistency: false} being the default in testing (at least for the jstests/core/ directory. That all being said, I'm still struggling to see why this test cleanup is worth doing over any other JavaScript cleanup we could also do (looking at ReplSetTest and ShardingTest). |
| Comment by Judah Schvimer [ 15/Apr/20 ] |
|
My only reasoning is to use defaults when possible because it's the mostly likely mode of user operation and it makes tests less complex. It also makes it clear when the causalConsistency setting is intentional vs. arbitrary. If there is a strong argument in the opposite direction, I'd suggest we just change the default in our tests to causalConsistency: false. |
| Comment by Max Hirschhorn [ 15/Apr/20 ] |
|
maria.vankeulen, judah.schvimer, could you expound on the motivation and benefit of removing {causalConsistency: false} when starting sessions for transactions testing? It isn't clear to me why if the test doesn't rely on causal consistency that we'd want necessarily use it anyway. |