[SERVER-63851] Use originating command for some expression context flags instead. Created: 18/Feb/22  Updated: 29/Oct/23  Resolved: 13/Sep/22

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

Type: Task Priority: Minor - P4
Reporter: Mickey Winters Assignee: Alberto Massari
Resolution: Fixed Votes: 1
Labels: quick-tech-debt
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Sprint: QE 2022-05-30, QE 2022-06-13, QE 2022-06-27, QE 2022-07-11, QE 2022-09-19
Participants:

 Description   

There are some expression context flags like allowDiskUse, fromMongos, needsMerge, bypassDocumentValidation, and hasWhere flags that are set based on things that are in the original command. Instead of storing these extra flags we could just have a reference to the original command and code could check that, or we could turn the flags into getters like fromMongos(). We already store the original command in cursors to begin with and even have an originalCommand field on the expression context. it's just not set a lot of the time. A downside to this approach is during testing the originalCommand would have to be updated instead of just setting a bool to true.



 Comments   
Comment by Githook User [ 13/Sep/22 ]

Author:

{'name': 'Alberto Massari', 'email': 'alberto.massari@mongodb.com', 'username': 'albymassari'}

Message: SERVER-63851 Derive ExecutionContext flags from OperationContext
Branch: master
https://github.com/mongodb/mongo/commit/c70bc17083cc73c2b2d32b4fc693fe22c58a863d

Comment by Mickey Winters [ 08/Sep/22 ]

alberto.massari@mongodb.com that sounds like it could work. I'm not sure if there are any flags that change dynamically during query execution, so this may not work for all flags. Any flags that are essential determined by the originating command definitely could be set in the constructor. Especially ones like fromMongos which is set if the originating command has the flag fromMongos: true

Comment by Alberto Massari [ 08/Sep/22 ]

mickey.winters@mongodb.com is there a set of flags whose value can change dynamically? Because we could change the three constructors to use the provided opCtx to fill the local flag using the source command, and leave to the caller the responsibility of manually setting the ones that cannot be inferred from opCtx

Comment by Mickey Winters [ 07/Mar/22 ]

Having flags like this can be bug prone, as shown in https://jira.mongodb.org/browse/SERVER-61769 a engineer could easily forget to appropriately set a flag leading to an error. in the ticket I linked instead of setting a flag in the expCtx it checks the source (the opCtx instead of the originating command in this case).
The other argument for this is there may be one off cases where we need to know something about the originating request but only have access to the expCtx. here I had to look at the originating command and set the flag just to have that information to be used in 1 other place: here. if the expression context had a reference to the originating command we wouldn't have to add new flags for one off cases like this.

Comment by Kyle Suarez [ 01/Mar/22 ]

mickey.winters, we discussed this at the triage meeting, but it wasn't immediately obvious that we should schedule this. Can you give us the quick "elevator pitch" for this refactor?

Also, david.storch raised a concern that this ticket should be careful to not e.g. parse the originating command twice.

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