[SERVER-43886] Check if the index was dropped after locking the weak_ptr to the IndexCatalogEntry in RequiresIndexStage and RequiresAllIndicesStage Created: 08/Oct/19 Updated: 29/Oct/23 Resolved: 10/Oct/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Querying, Storage |
| Affects Version/s: | 4.3.1 |
| Fix Version/s: | 4.3.1 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Gregory Wlodarek | Assignee: | Gregory Wlodarek |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Sprint: | Execution Team 2019-10-21 |
| Participants: |
| Description |
|
While working on the background validation project, it came to my attention that requires_index_stage.h holds a weak_ptr to a shared_ptr of IndexCatalogEntry. This is used to detect when the underlying index entry has been destroyed. However, background collection validation holds a shared_ptr to the IndexCatalogEntries it’s validating even when they’ve been removed from the IndexCatalog while periodically yielding locks during validation. This seems unsafe for requires_index_stage.h to rely on a weak_ptr now. I was thinking if we should change the weak_ptr to a shared_ptr and check the isDropped() flag on the IndexCatalogEntry that was added as part of the background validation project here. |
| Comments |
| Comment by Githook User [ 10/Oct/19 ] |
|
Author: {'name': 'Gregory Wlodarek', 'username': 'GWlodarek', 'email': 'gregory.wlodarek@mongodb.com'}Message: |
| Comment by David Storch [ 08/Oct/19 ] |
|
We believe that this was a regression introduced in 4.3 development. However, if the background validation work gets backported, then this fix must get backported too. |
| Comment by Charlie Swanson [ 08/Oct/19 ] |
|
One clarification: It certainly seems like we should be checking 'isDropped()' here, but is the weak pointer really a problem? Should a yielded query really prevent the destruction of the IndexCatalogEntry? I'm happy either way but I don't see the motivation for changing it. |
| Comment by Gregory Wlodarek [ 08/Oct/19 ] |
|
Clarification from Slack: background validation holds shared_ptrs to IndexCatalogEntries even if they've been dropped. So if the index is dropped and validation was running, the shared_ptr would still exist even though the index was dropped. |