[SERVER-42484] May not be inside required WriteUnitOfWork when writing multikey index keys during initial sync data cloning Created: 29/Jul/19 Updated: 29/Oct/23 Resolved: 25/Sep/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Replication |
| Affects Version/s: | 4.2.0-rc4 |
| Fix Version/s: | 4.2.1, 4.3.1, 4.0.15 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | William Schultz (Inactive) | Assignee: | William Schultz (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||||||||||||||||
| Backport Requested: |
v4.2, v4.0
|
||||||||||||||||||||||||||||||||||||||
| Steps To Reproduce: |
|
||||||||||||||||||||||||||||||||||||||
| Sprint: | Repl 2019-09-09, Repl 2019-09-23, Repl 2019-10-07 | ||||||||||||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||||||||||||
| Case: | (copied to CRM) | ||||||||||||||||||||||||||||||||||||||
| Linked BF Score: | 0 | ||||||||||||||||||||||||||||||||||||||
| Description |
|
During initial sync collection cloning, we insert documents via the CollectionBulkLoader, which also inserts index keys. When we insert keys into an external sorter here, we are not inside a WriteUnitOfWork. Normally this is safe, since we will only be doing in-memory writes through the BulkBuilder, but if we are in FCV=4.0 and the index build is done using the Background method, then we will not use the bulk builder, and instead go through the IndexAccessMethod, which may in fact write to the storage layer if we are updating a multikey index. We need to make sure that we are enclosing these writes inside a WUOW in case they end up writing to storage. |
| Comments |
| Comment by Githook User [ 13/Jan/20 ] |
|
Author: {'name': 'William Schultz', 'email': 'william.schultz@mongodb.com', 'username': 'will62794'}Message: (cherry picked from commit c41af3e315a115b04a53706837d36a729d174fc3) |
| Comment by Githook User [ 01/Oct/19 ] |
|
Author: {'name': 'William Schultz', 'username': 'will62794', 'email': 'william.schultz@mongodb.com'}Message: (cherry picked from commit c41af3e315a115b04a53706837d36a729d174fc3) |
| Comment by Githook User [ 25/Sep/19 ] |
|
Author: {'username': 'will62794', 'email': 'william.schultz@mongodb.com', 'name': 'William Schultz'}Message: |
| Comment by William Schultz (Inactive) [ 24/Sep/19 ] |
|
For reference, this bug does also appear on master if you explicitly set enableHybridIndexBuilds=false. This is the repro: initial_sync_multikey.js |
| Comment by Judah Schvimer [ 24/Sep/19 ] |
|
In 4.2+ case 1 is all that applies. enableHybridIndexBuilds is true by default in 4.2+, so by default, in 4.2+ we always use WT bulk cursor. I don't think this is something we would want to backport though. |
| Comment by Suganthi Mani [ 23/Sep/19 ] |
|
Few thoughts from the observation for index building (irrespective of multi-index keys) during initial sync. case 1 - If it is FCV 4.2+ & mongod binary 4.2+ , then it first checks the enableHybridIndexBuilds (start-up/runtime server parameter). If enableHybridIndexBuilds is false, then it uses the value set in "background" param in the index spec received from the sync source. So, for case 1 & case 2, we have a possibility of running background index build. This results in not using the WT bulk cursor. As, we currently use WT bulk cursor only for foreground and hybrid index building. But, I feel during index building during initial sync should always use WT bulk cursor (i.e foreground or hybrid) for better performance. milkie any thoughts. |
| Comment by William Schultz (Inactive) [ 10/Sep/19 ] |
|
The contract here seems a bit unclear, since the MultiIndexBlock::insert method claims that it must be called inside a WriteUnitOfWork, but the changes from |
| Comment by Suganthi Mani [ 01/Aug/19 ] |
|
william.schultz It seems like its not problem on 4.0 because MultiIndexBlockImpl constructor sets _buildInBackground to false. As a result, MultiIndexBlockImpl::init will disable background index building even if index spec contains the "background" field as true. So, it seems we always do bulk index building for database cloning phase of initial sync. |
| Comment by Kelsey Schubert [ 30/Jul/19 ] |
|
My inclination is towards stability of the GA release and to let this wait until 4.2.1. |
| Comment by Judah Schvimer [ 30/Jul/19 ] |
|
We may only need to do this on 4.2 and earlier. Doing it on master is an open question. kelsey.schubert, in FCV 4.0 doing a multikey index build during initial sync data cloning will invariant. Should we do this before 4.2.0, or wait for 4.2.1? |
| Comment by William Schultz (Inactive) [ 29/Jul/19 ] |
|
This may also be an issue on 4.0 after BACKPORT-4730 is completed. |