[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:
Backports
Depends
Related
related to SERVER-50188 Have multiple resmoke setParameters c... Closed
related to SERVER-61460 Resmoke should merge config_svr optio... Closed
is related to SERVER-49790 Disable index build commitQuorum on "... Closed
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:

// resmoke.py
[MongoDFixture:job0] {"t":{"$date":"2020-04-13T14:13:05.638-04:00"},"s":"I", "c":"CONTROL", "id":21951,  "ctx":"initandlisten","msg":"Options set by command line","attr":{"options":{"net":{"port":20000},"replication":{"enableMajorityReadConcern":true},
"setParameter":... "logComponentVerbosity":"{'command': 2}", ...,
"storage":{"dbPath":"/data/db/job0/resmoke"}}}} 

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 SERVER-47510



 Comments   
Comment by Githook User [ 01/Jun/21 ]

Author:

{'name': 'Carl Raiden Worley', 'email': 'carl.worley@10gen.com', 'username': 'aggrand'}

Message: SERVER-47509 resmoke accepts multiple mongodSetParameters options but only uses the last one

(cherry picked from commit a224fd19449424a84c269c7eec798c05b6ea4474)
Branch: v4.2
https://github.com/mongodb/mongo/commit/17506820c39a3c4312fe727ffe984894f1133941

Comment by Githook User [ 19/May/21 ]

Author:

{'name': 'Jason Chan', 'email': 'jason.chan@10gen.com', 'username': 'jasonjhchan'}

Message: SERVER-47509 resmoke accepts multiple "mongodSetParameters" options but only uses the last one.
This commit also cherry-picks the following.
SERVER-39448 Make resmoke pass TestData.setParametersMongos to mongo shell as a JavaScript object
(cherry picked from commit f998d1f6bd1d74a815e1bbe6e984c8e73da8398d)
Branch: v4.0
https://github.com/mongodb/mongo/commit/27b2ba46fe86aff0cf8170f35301bf43bda99204

Comment by Jason Chan [ 14/May/21 ]

Requesting backport of this to v4.2 and v4.0 so we can do SERVER-56952.

 

EDIT: v4.0 might be a stretch since it looks like these changes went in after STM did a rewrite of the argparse (SERVER-46769) which was only backported to v4.2.

Comment by Githook User [ 18/Feb/21 ]

Author:

{'name': 'Carl Raiden Worley', 'email': 'carl.worley@10gen.com', 'username': 'aggrand'}

Message: SERVER-47509 resmoke accepts multiple mongodSetParameters options but only uses the last one
Branch: v4.4
https://github.com/mongodb/mongo/commit/a224fd19449424a84c269c7eec798c05b6ea4474

Comment by Githook User [ 05/Aug/20 ]

Author:

{'name': 'Daniel Gottlieb', 'email': 'daniel.gottlieb@mongodb.com', 'username': 'dgottlieb'}

Message: SERVER-47509: Correct merge conflict resoluation
Branch: master
https://github.com/mongodb/mongo/commit/6da20f433cc8a6ae47664f89085ee8873817d651

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: SERVER-47509: Have resmoke better cope with multiple set parameters.
Branch: master
https://github.com/mongodb/mongo/commit/52f4d4d869dca67d7ed22e8956d9fb56a8a79944

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:

  • I don't think having later mongodSetParameters override earlier ones is appropriate. The idea is sound, but in practice our current evergreen setup does not emit mongodSetParameters in the order that I would declare safe. In the BF louis.williams had, the rollback fuzzer task wants to run with the commit quorum turned off. The test will otherwise fail if it's on. The competing set parameter was the eMRC=off variant* setting an unrelated setParameter (oplog application enforces steady state constraints):

     [2020/04/07 06:30:02.794] echo "--suites=rollback_fuzzer --mongodSetParameters='{logComponentVerbosity: {command: 2}, enableIndexBuildCommitQuorum: false}' --majorityReadConcern=off --excludeWithAnyTags=requires_majority_read_concern,uses_prepare_transaction,uses_multi_shard_transaction,uses_atclustertime --mongodSetParameters="{oplogApplicationEnforcesSteadyStateConstraints: false}"" | grep -q storageEngineCacheSizeGB
    

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.

  • Not all setParameters are equal (e.g: logComponentVerbosity), and some are more equal than at first glance. When resmoke passes setParameters on the command line to the mongo[ds] instance, the resmoke inputs get merged with the --suite inputs.

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 ]

It might be an issue with the Python option parser ignoring duplicates.

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 SERVER-27408 by switching to use action="append".

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.

Generated at Thu Feb 08 05:14:23 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.