[SERVER-34774] Converting an index to multikey is not visible within the same txn, causing incorrect data to be returned Created: 01/May/18 Updated: 29/Oct/23 Resolved: 16/May/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Index Maintenance |
| Affects Version/s: | 3.7.9 |
| Fix Version/s: | 4.0.0-rc0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Robert Guo (Inactive) | Assignee: | Daniel Gottlieb (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||||||
| Steps To Reproduce: |
|
||||||||||||||||||||||||||||
| Sprint: | Storage NYC 2018-05-21 | ||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||
| Linked BF Score: | 36 | ||||||||||||||||||||||||||||
| Description |
|
The update() call in the repro script is not visible, causing the assert.eq on the next line to fail:
|
| Comments |
| Comment by Githook User [ 16/May/18 ] | ||||||||||||||||||||||||
|
Author: {'email': 'daniel.gottlieb@mongodb.com', 'username': 'dgottlieb', 'name': 'Daniel Gottlieb'}Message: | ||||||||||||||||||||||||
| Comment by Robert Guo (Inactive) [ 02/May/18 ] | ||||||||||||||||||||||||
|
There's a related symptom for geo indexes that Max pointed out could have the same underlying cause. Here is the repro for future reference.
| ||||||||||||||||||||||||
| Comment by Daniel Gottlieb (Inactive) [ 01/May/18 ] | ||||||||||||||||||||||||
To be clear, we haven't modified the on-disk index until the transaction commits. Outside readers are fine, they cannot view any multikey data. The problem with prematurely setting the in-memory multikey variable outside of the onCommit is that this prevents a future writer from trying to set the index as multikey, ultimately resulting in a subtle race that can result in data corruption (the multikey write must occur at a time <= the first write that makes an index multikey). It's possible for the future writer to actually get its write into the oplog earlier than the write that contains the multikey change. | ||||||||||||||||||||||||
| Comment by Daniel Gottlieb (Inactive) [ 01/May/18 ] | ||||||||||||||||||||||||
|
One clarification, the query is not returning stale data, but rather, when an index is multikey, the covered index optimization cannot be used. A better demonstration of what's happening is to instead perform an update that sets the value to of a to [1,2,3]. That gives us the following output when performing an index scan:
Result:
Another representation is to also include the record id's in the result (which I think removes the covered index optimization):
Result:
| ||||||||||||||||||||||||
| Comment by Kyle Suarez [ 01/May/18 ] | ||||||||||||||||||||||||
|
What is the proposed solution? It seems like modifying the index itself would conflict with concurrent reads outside of the transaction. Should we stash multikey index changes as part of the transaction metadata? | ||||||||||||||||||||||||
| Comment by Eric Milkie [ 01/May/18 ] | ||||||||||||||||||||||||
|
The multikey flag is set in a commit handler; this is conflicting with the new 4.0 ability to do a write and then read your write from the same transaction. The query system needs to know when reading from an index whether the keys are possibly multikey or not, in order to correctly return covered data from such an index. | ||||||||||||||||||||||||
| Comment by Eric Milkie [ 01/May/18 ] | ||||||||||||||||||||||||
|
Dan and I think we know what's going on here. | ||||||||||||||||||||||||
| Comment by Kyle Suarez [ 01/May/18 ] | ||||||||||||||||||||||||
|
Let's see what the Query Team has to say at the Needs Triage meeting. I feel like the Repl Team has a lot on its plate right now; if the Query Team has resources to look into the failure, then let's go for it. | ||||||||||||||||||||||||
| Comment by Daniel Gottlieb (Inactive) [ 01/May/18 ] | ||||||||||||||||||||||||
|
milkie and I just took a look at this. When thinking about transactions and what how RTT does catalog writes for multikey, I was expecting a bug, but a different manifestation than what this repro is showing. I do think this is, in essence, a transactions problem. If replication is better equipped to diagnose that, perhaps your suggestion is better. FWIW, I'm running a modified script to see if my hypothetical bug manifests as expected. | ||||||||||||||||||||||||
| Comment by Kyle Suarez [ 01/May/18 ] | ||||||||||||||||||||||||
|
daniel.gottlieb, judah.schvimer: should we triage this as the Repl Team first before handing it off to Query? |