[SERVER-71467] Dont run feature flag specific tests for multiversion testing Created: 17/Nov/22 Updated: 29/Oct/23 Resolved: 10/Jan/23 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 6.0.4, 6.3.0-rc0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Gregory Noma | Assignee: | Tausif Rahman (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Assigned Teams: |
Server Development Platform
|
||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||
| Backport Requested: |
v6.2, v6.0
|
||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||
| Linked BF Score: | 157 | ||||||||||||||||||||||||
| Description |
|
In all-feature-flag multiversion testing, in the global_index_rollback.js test the featureFlagGlobalIndexes feature flag is not enabled on the 6.2 nodes which causes the test to fail due to "no such command: '_shardsvrCreateGlobalIndex'". The above is how this issue was surfaced. We shouldn't run feature flag specific tests during multiversion testing for 2 reasons. (1) The feature flag c++ code is gated by FeatureFlag::isEnabled(serverGlobalParams.featureCompatibility) & multiversion testing's featureCompatibilityVersion will always be lower than the required FCV for a non-default feature flag. Therefore, the feature flag code can never run with the given FCV. This will always result in failures for featureFlag specific tests. There is a resmoke argument specifically for this use case: --runNoFeatureFlagTests. We should automatically add this when we are running multiversion testing in CI. |
| Comments |
| Comment by Githook User [ 14/Dec/22 ] |
|
Author: {'name': 'Tausif Rahman', 'email': 'tausif.rahman@mongodb.com', 'username': 'trahman1318'}Message: (cherry picked from commit 58818ad6cc1445841f43e02df4e2b4866e591281) |
| Comment by Tausif Rahman (Inactive) [ 13/Dec/22 ] |
|
need to do backports |
| Comment by Githook User [ 12/Dec/22 ] |
|
Author: {'name': 'Tausif Rahman', 'email': 'tausif.rahman@mongodb.com', 'username': 'trahman1318'}Message: |
| Comment by Tausif Rahman (Inactive) [ 07/Dec/22 ] |
|
Ah okay, that makes sense. Thanks for clarifying max.hirschhorn@mongodb.com. I think this is best suited for --excludeWithAnyTags=featureFlagXX,featureFlagYY, etc. |
| Comment by Max Hirschhorn [ 06/Dec/22 ] |
|
Thanks for the question tausif.rahman@mongodb.com. The multiversion testing story we have didn't get a holistic design so I think it is fair to question some of the utility. I would say it is important to continue running the multiversion tests on the "all feature flags" build variants. The purpose of the "all feature flags" build variant is to reflect what would happen if all in-development feature flags were released (defaulted to on) today. The multiversion tests run in a lower FCV and so even with --setParameter featureFlagXX, C++ code which is appropriately using FeatureFlag::isEnabled(serverGlobalParams.featureCompatibility) will continue to not execute. The C++ code would only execute after {setFeatureCompatibilityVersion: kLatest} is run. And hence there is the odd detail that none of the JavaScript tests tagged with featureFlagXX are valid to run because the explicit tests for the feature would be expected to fail. However, if a feature flag is ignoring the feature compatibility version (e.g. perhaps it is a feature independent from replication or other cases found by That is to say, running the multiversion tests on the "all feature flags" build variants is the only way I see to catch a feature inappropriately not consulting the FCV value prior to the commit which attempts to enable the feature by default. And the primary objective of our multiversion testing is to ensure enabling the feature by default is a non-event where we have been doing all of the required testing commit-over-commit the whole time. |
| Comment by Tausif Rahman (Inactive) [ 06/Dec/22 ] |
|
max.hirschhorn@mongodb.com I'm looking at this again & I'm curious if we should even be running multiversion testing with feature flags enabled. "All feature flags" enabled will enable all feature flags that do not yet have a default version. These features are not available in past versions & I don't think it serves any purpose to enable these on the latest version just to disable any tests that use those feature flags (since those test will definitely fail on previous versions). By not running this in an "all feature flags" variant, the tests will be naturally excluded/included. If we take featureFlagGlobalIndexes as an example – since featureFlagGlobalIndexes does not have a default version it will not be enabled & any tests with the featureFlagGlobalIndexes tag will be excluded from running. This allows multiversion testing to successfully exclude unsupported feature flag specific testing. When featureFlagGlobalIndexes eventually gets a default version – say 6.3 – all featureFlagGlobalIndexes tags will be updated to requires_fcv_63. If master is 6.3 this feature flag is enabled by default & any old featureFlagGlobalIndexes tests would be fair game, but they are blocked from running in multiversion testing because of the requires_fcv_63 tag. Do you think not running multiversion tests in "all feature flags" variants is a better solution? |
| Comment by Tausif Rahman (Inactive) [ 06/Dec/22 ] |
|
huayu.ouyang@mongodb.com tagging with requires_fcv_63 will prevent the test from running in multiversion configurations until 6.3 is released. That's a sufficient workaround for now, but as discussed in Slack, it is technically "wrong" since that feature flag does not yet have a version. Long term, the above plan will make it so that we don't need to use requires_fcv_63 in a hacky way. |
| Comment by Max Hirschhorn [ 06/Dec/22 ] |
|
Yes, I believe using requires_fcv_63 on the master branch in the interim is also a solution and is being pursued in huayu.ouyang@mongodb.com, I had (wishfully) interpreted |
| Comment by Huayu Ouyang [ 06/Dec/22 ] |
|
tausif.rahman@mongodb.com I was also under the impression that tagging the test with featureFlagGlobalIndexes and requires_fcv_63 should have excluded the test from being run in multiversion configurations. Does that not work? max.hirschhorn@mongodb.com Also, sorry I believe the title/description of |
| Comment by Max Hirschhorn [ 06/Dec/22 ] |
|
I don't think server_feature_flags.idl should be sourced directly for how resmoke learns of which feature flags exist and which are not enabled by default. That metadata should probably instead be generated by buildscripts/idl/gen_all_feature_flag_list.py and persisted in the all_feature_flags.txt file. tausif.rahman@mongodb.com, what you describe about adding --excludeWithAnyTags for each featureFlagXX, featureFlagYY, etc. makes sense to me. I was under the impression that a combination of the work done under CC huayu.ouyang@mongodb.com because it looks like addressing |
| Comment by Tausif Rahman (Inactive) [ 06/Dec/22 ] |
|
On the testing infra side, we can probably parse the server_feature_flags.idl file for any feature flags that are not enabled by default and automatically add an --excludeWithAnyTags=featureFlag1,featureFlag2,featureFlag3 for multiversion testing. Whenever the feature flag is eventually enabled by default on a server version, the tests will no longer be excluded. In the interim, we can add checks in the tests as is done here. This is supported & described in the 3rd bullet in the readme: https://github.com/mongodb/mongo/blob/085aa724abcba8481860109593c70620915069e2/jstests/sharding/txn_addingParticipantParameter.js#L53-L59 Any concerns with that approach max.hirschhorn@mongodb.com? |
| Comment by Max Hirschhorn [ 05/Dec/22 ] |
The featureFlagGlobalIndexes feature flag is not enabled by default in any server version still. MongoDB 6.2, which is a production version, must therefore not support running the _shardsvrCreateGlobalIndex command. Speaking for an arbitrary feature, because the feature flag is disabled, there is no reason to expect the MongoDB 6.2 behavior is consistent with the MongoDB 6.3 behavior. It could just as well be that the feature is only partially implemented in MongoDB 6.2. Atlas isn't ever going to specify --setParameter featureFlagGlobalIndexes=true for the 6.2 release and so it would misleading for our testing infrastructure to specify it. It would be most appropriate for the testing infrastructure to skip any tests which are dependent on the featureFlagGlobalIndexes feature flag being enabled. |
| Comment by Matt Kneiser [ 22/Nov/22 ] |
|
All feature flags are not enabled on prior versions in the multiversion testing. Why? |