[SERVER-69237] Make the preImage document always available to CollectionUpdateArgs from UpdateStage Created: 29/Aug/22 Updated: 29/Oct/23 Resolved: 18/Nov/22 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Write Ops |
| Affects Version/s: | None |
| Fix Version/s: | 6.3.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Max Hirschhorn | Assignee: | Israel Hsu |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||
| Sprint: | Sharding NYC 2022-10-31, Sharding NYC 2022-11-14, Sharding NYC 2022-11-28 | ||||||||||||
| Participants: | |||||||||||||
| Description |
|
CollectionUpdateArgs currently has a boost::optional<BSONObj> preImageDoc member representing the state of the document before the update occurred. For storage engines supporting document-level concurrency, the preImage document is always copied out of the storage engine before applying the update. This means that, since the removal of the MMAPv1 storage engine in MongoDB 4.2, CollectionUpdateArgs::preImageDoc can be non-optional.
As part of these changes we should consider simplifying the places which attempted to carefully get an owned copy of the oldObj and newObj and instead make owned BSONObjs for them available inside and outside UpdateStage in an obvious way. Some areas which are ripe for refactoring include:
Note that CollectionUpdateArgs::storeDocOption should continue to be StoreDocOption::kNone when the preImage document doesn't need to be stored in config.image_collection, etc. Side note: In the MMAPv1 storage engine, in-place updates were truly applied in-place meaning that the address of oldObj and the address of newObj were the same address. |
| Comments |
| Comment by Israel Hsu [ 18/Nov/22 ] |
|
Code reviews: Merged PRs: |
| Comment by Githook User [ 18/Nov/22 ] |
|
Author: {'name': 'Israel Hsu', 'email': 'israel.hsu@mongodb.com', 'username': 'israelhsu'}Message: |
| Comment by Githook User [ 18/Nov/22 ] |
|
Author: {'name': 'Israel Hsu', 'email': 'israel.hsu@mongodb.com', 'username': 'israelhsu'}Message: |
| Comment by Israel Hsu [ 19/Oct/22 ] |
|
I chatted with mihai.andrei@mongodb.com. The plan is to remove the conditions that prevent preImageDoc from being assigned to CollectionUpdateArgs (but still leave that member as boost::optional, at least temporarily.) Testing will be changed to ensure that preImageDoc is initialized (not boost::none), so failing tests should be able to point to any code paths that leave it uninitialized. |
| Comment by Israel Hsu [ 17/Oct/22 ] |
|
Status: Examined all the cases where the assignment of preImageDoc is dependent on some condition. Question 1: Is it sufficient to replace the conditional assignment with just an assignment? Question 2: Are there any code paths through Update where the preImageDoc is never set? Testing: Identify the code paths that originally left the preImageDoc unset, and make sure that tests exercise those paths and confirm that preImageDoc is set. |
| Comment by Brenda Rodriguez [ 20/Sep/22 ] |
|
max.hirschhorn@mongodb.com we are adding this to the QE Backlog, but if you have an engineer who can take it, we can review it. |