[SERVER-47509] resmoke accepts multiple "mongodSetParameters" options but only uses the last one Created: 13/Apr/20 Updated: 29/Oct/23 Resolved: 05/Aug/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 4.7.0, 4.4.5, 4.0.25, 4.2.15 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Louis Williams | Assignee: | Daniel Gottlieb (Inactive) |
| Resolution: | Fixed | Votes: | 3 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||
| Backport Requested: |
v4.4, v4.2, v4.0
|
||||||||||||||||||||||||
| Sprint: | STM 2020-08-10 | ||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||
| Linked BF Score: | 50 | ||||||||||||||||||||||||
| Story Points: | 2 | ||||||||||||||||||||||||
| Description |
|
For example: resmoke --suite=core --mongodSetParameters='{enableIndexBuildCommitQuorum: false}' --mongodSetParameters='{logComponentVerbosity: {command: 2'}} Will override the first argument and only apply the second:
This causes problems in testing because the "majority read concern off" suite enables custom setParameters but runs suites like the rollback_fuzzer that enables its own setParameters. The effect is that the builder's "mongodSetParameters" overrides those of the test suite. Also revert the changes in |
| Comments |
| Comment by Githook User [ 01/Jun/21 ] | |
|
Author: {'name': 'Carl Raiden Worley', 'email': 'carl.worley@10gen.com', 'username': 'aggrand'}Message: (cherry picked from commit a224fd19449424a84c269c7eec798c05b6ea4474) | |
| Comment by Githook User [ 19/May/21 ] | |
|
Author: {'name': 'Jason Chan', 'email': 'jason.chan@10gen.com', 'username': 'jasonjhchan'}Message: | |
| Comment by Jason Chan [ 14/May/21 ] | |
|
Requesting backport of this to v4.2 and v4.0 so we can do
EDIT: v4.0 might be a stretch since it looks like these changes went in after STM did a rewrite of the argparse ( | |
| Comment by Githook User [ 18/Feb/21 ] | |
|
Author: {'name': 'Carl Raiden Worley', 'email': 'carl.worley@10gen.com', 'username': 'aggrand'}Message: | |
| Comment by Githook User [ 05/Aug/20 ] | |
|
Author: {'name': 'Daniel Gottlieb', 'email': 'daniel.gottlieb@mongodb.com', 'username': 'dgottlieb'}Message: | |
| Comment by Daniel Gottlieb (Inactive) [ 05/Aug/20 ] | |
|
Specifying multiple `--mongodSetParameters` or `--mongosSetParameters` in resmoke no longer silently throws away some of the inputs. Resmoke will parse the value (a yaml string) into a map/dictionary and merge the two maps together. If the maps overlap on any top-level keys, resmoke will error out, even if the values are the same. There were no cases of duplicated set parameter keys in our evergreen configuration, so it seemed best to not add code to accept a case that didn't exist. | |
| Comment by Githook User [ 05/Aug/20 ] | |
|
Author: {'name': 'Daniel Gottlieb', 'email': 'daniel.gottlieb@mongodb.com', 'username': 'dgottlieb'}Message: | |
| Comment by Daniel Gottlieb (Inactive) [ 24/Jul/20 ] | |
|
I've put up a CR that I'm confident will make this a problem of the past. That said, there were some differing opinions on what the correct behavior is; particularly when the same top-level `mongo{d,s}SetParameter` key is defined twice. This ticket is simple enough (regardless of which decision is made) that I didn't want to complicate/draw out the time to close this ticket out by inviting stakeholders to some sort of roundtable to discuss. However, I did consider both cases and used the resmoke code to help tease out some nuances/cases that I wasn't really aware about prior to taking on the ticket (discussed below). I additionally did some (not completely exhaustive) research to see if there even were conflicts today. The only conflicts were intentional duplications of disabling the index build commit quorum (the problem that spawned this ticket). Given there will no longer be any conflicts as part of this ticket, the decision about the right behavior is much more hypothetical. TL;DR: The current patch dictates multiple setParameters on the command line will merge – duplicated keys (on different setParameters, not within the same one) will error. A suite's setParameters will be overridden by a resmoke CLI mongo[ds]SetParameters. Observations/Conclusions/Justifications:
Note that the variant's set parameter comes after the task's setParameter – declaring that the latest setParameter wins would be saying that variant setParameters should override individual task setParameters. Overriding, IMO only makes sense if the "lowest level" structure wins (patch run -> variant -> task). If resmoke were to make an assumption on ordering of setParameters, it should be the last one wins, not doing so seems unacceptably fragile. But to repeat, there are no current cases (of the variants I turned on in the patch build) where multiple setParameters conflict.
Consider the case of the "sharding_rs_matching_match_busiest_node" suite. Presumably that suite could be expressed as a single evergreen task of resmoke run --mongodSetParameters='{"ShardingTaskExecutorPoolReplicaSetMatching": "matchBusiest"}' jstests/sharding/{,change_streams/,query/*}.js. We can arguably declare some equivalence between a suite's setParameters and the setParameters on the resmoke command line. By that reasoning, it would be "more right" to disallow setParameters that conflict on keys between a suite's definition and the command line. I don't think that's necessarily appropriate either. It's exceptionally common for suites to override logComponentVerbosity. A user adding mongo[ds]SetParameter on the command line should always be able to change the logComponentVerbosity. I did consider disallowing suites + resmoke to conflict on duplicate keys, but leaving an exception for logComponentVerbosity. In the end that felt like a lot of work that was perhaps too prescriptive (should a user not* be able to override numInitialSyncAttempts in a local run?) And to repeat, unless I did something wrong (certainly possible – there's a lot of unrelated red boxes to sift through), this is all hypothetical. | |
| Comment by Max Hirschhorn [ 14/Apr/20 ] | |
The --mongodSetParameters and --mongosSetParameters command line options use the default action="store". We faced a similar issue in the past with --excludeWithAnyTags being specified on both a build variant definition and a task definition. It was addressed in Having top-level keys in a later --mongodSetParameters blob override top-level keys in an --mongodSetParameters blob seems easy enough to reason about. This is effectively what those command line options means for set_parameters expressed in the YAML suite file. I'd still say attempting to do any kind of "deep merging" (e.g. around log component verbosity) would be asking for trouble. |