[SERVER-80903] Audit gFeatureFlagBucketUnpackWithSort Created: 08/Sep/23  Updated: 20/Sep/23  Resolved: 20/Sep/23

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

Type: Task Priority: Major - P3
Reporter: Kyle Suarez Assignee: David Percy
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by SERVER-80904 Audit gFeatureFlagBucketUnpackWithSort Closed
Related
is related to SERVER-79317 Provide more documentation and helper... Closed
Assigned Teams:
Query Optimization
Backwards Compatibility: Fully Compatible
Sprint: QO 2023-10-02
Participants:

 Description   

This ticket has been split from an audit of all Query 7.0 feature flags. This ticket is a request to audit gFeatureFlagBucketUnpackWithSort.

Intial sync can temporarily reset the fcv value to uninitialized and sets the new value afterwards. This can cause call sites trying to inspect the fcv value to hit this invariant. We need to audit feature flag usage and determine which should do one of the following:

  • It can never be called when initial sync is running. So do nothing. Note that this can be tricky to prove as we once thought the catalog cache loader can never be run while initial sync is happening but it can.
  • It is safe to ignore fcv version so use isEnabledAndIgnoreFCVUnsafe instead
  • It is safe to turn off the feature anytime (even if feature flag is actually on) so use isEnabledUseDefaultFCVWhenUninitialized instead.
  • Special logic is needed if fcv is not initialized


 Comments   
Comment by David Percy [ 19/Sep/23 ]

This feature flag enables a query optimization for time-series collections. It doesn't create any persistent data because it's only a query, and it doesn't send messages to other nodes because this optimization only applies within each mongod.

When a node is doing initial sync, I believe it doesn't count as a secondary, because the data is not consistent until it's caught up.  So I wouldn't expect user queries to be routed there, and I don't think it matters whether any particular query optimization is used.  But to defend against malicious inputs let's assume it can happen, and make sure we don't trigger the invariant() check.

This feature flag exists on 3 branches: v5.0, v6.0, and v7.0. Here are all the occurrences of "gFeatureFlagBucketUnpackWithSort":

  • It's declared in the IDL.
  • In document_source_sort.cpp we check the flag when registering the parser for $_internalBoundedSort. This registration happens during startup, so we expect FCV to be uninitialized. The appropriate thing to do is consider the feature to be enabled, so we register the parser, which can check the actual FCV later when parsing each request. On all three branches we are calling either isEnabledAndIgnoreFCV() or isEnabledAndIgnoreFCVUnsafeAtStartup(), so this is good.
  • In pipeline_d.cpp we check the flag to decide whether to try the optimization. These checks all either use isEnabledAndIgnoreFCV() or isVersionInitialized() && isEnabled(). While this is not very consistent, it won't hit the invariant.

Going forward, on branches v7.1 and later this feature flag has been removed (SERVER-67588).

Generated at Thu Feb 08 06:44:54 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.