[SERVER-57511] Only the index build operation should remove the indexBuilds document on index build abort/commit Created: 07/Jun/21 Updated: 06/Dec/22 Resolved: 10/Jun/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Dianna Hohensee (Inactive) | Assignee: | Backlog - Storage Execution Team |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | techdebt | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Assigned Teams: |
Storage Execution
|
||||||||
| Participants: | |||||||||
| Description |
|
dropIndexes runs this code to remove the indexBuilds document used by an in-progress index build. This can lead to strange (internal system details) user index build errors like the one identified in SERVER-56279. Further work is necessary to ensure that the index build thread, when aborted, always cleans up its own indexBuilds document. |
| Comments |
| Comment by Dianna Hohensee (Inactive) [ 08/Jun/21 ] |
|
Okay, it sounds like the secondary index build thread isn't listening for the abort/commit, and we don't have a means of otherwise running the clean up on the secondary. Then in lieu of making significant changes, we should better handle the error thrown to the user. |
| Comment by Louis Williams [ 08/Jun/21 ] |
|
I'm mostly thinking about case number 2. If an index build has replicated the startIndexBuild oplog entry, if the index build thread is interrupted for any reason, it needs to be guaranteed to replicate an abortIndexBuild oplog entry. We can't guarantee that it will be the case, for the reasons I state above. If it can't replicate (because of a step-down, for example), then we've just killed the index builder thread and left the catalog in an unfinished state. So a new primary might be able to commit/abort, but the old primary isn't building the index anymore. I could imagine a solution where we entirely restart an index build if the thread exits without cleaning up, but that would be quite an undertaking and we rejected that solution in favor of the alternative: forcing the caller to clean up. |
| Comment by Dianna Hohensee (Inactive) [ 08/Jun/21 ] |
|
louis.williams Oh, that's interesting. I would have thought that if Server A is primary, and building an index, and then is externally aborted and steps down before cleaning up, one of two things could happen: 1. Server A's startIndexBuilds oplog entry was not replicated, so therefore the persisted data gets rolled back. 2. Server A's startIndexBuilds oplog entry was replicated, and the new primary would clean up the index build. Is the current design because some writes didn't used to be timestamped?
Regarding SERVER-56279, we could patch for the NoMatchingDocument error. My concern with that fix is that it's a bandaid that might not stop other internal errors from popping up at the user level. We have identified the particular problem point, though, the async document deletion. So we could just manage the functions pertaining to accessing the document and choose wrap the error with a better explanation for the user. |
| Comment by Louis Williams [ 07/Jun/21 ] |
|
We very intentionally designed index builds so that the calling operation (i.e. dropIndexes), not the index build thread, is responsible for 1. taking locks 2. performing writes 3. interrupting the thread. The alternative, as you proposed, means we would have to deal with an entirely different set of problems. If an index build is interrupted and releases its locks, it may be unable to reacquire its locks and delete the catalog entry and replicate an oplog entry. Once the thread is interrupted or the node is no longer primary, the only option we have is to crash. Regarding SERVER-56279, would it be possible to instead suppress a NoMatchingDocument error? |