[SERVER-43948] Use Future-Promise pair to convey that an IndexCatalogEntry has been dropped/destructed Created: 10/Oct/19 Updated: 06/Dec/22 Resolved: 20/Jul/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Storage |
| 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 Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Storage Execution
|
| Participants: |
| Description |
|
Query code and background validation both hold pointers to IndexCatalogEntrys outside of locks, via the IndexCatalog::getEntryShared() function. This leads to problems when the index is either dropped or the catalog recreated for the restartCatalog cmd (open/closeCatalog), where the IndexCatalogEntry becomes unsafe, for example because it has IndexDescriptors pointing to memory that has been released with the catalog destruction. Query currently holds a weak_ptr to the IndexCatalogEntry, and depends upon no other callers to getSharedEntry, which is a very fragile dependency with no safeguards. With background validation, we now we have an isDropped function on the IndexCatalogEntry, along with a catalog generation number saved on the ServiceContext, in order to detect when an IndexCatalogEntry shared_ptr is no longer safe. Instead of the difficulties and subtleties above, I suggest a Future-Promise solution, wherein callers can get a Promise that notifies them when the IndexCatalogEntry is destructed for any reason, along with the appropriate error code. |
| Comments |
| Comment by Connie Chen [ 20/Jul/20 ] | ||||||||
|
We'll do this within the Lock Free Reads Project with shared pointers | ||||||||
| Comment by William Olaleye [ 24/Oct/19 ] | ||||||||
|
need to research what the correct solution is to solve this problem. | ||||||||
| Comment by Dianna Hohensee (Inactive) [ 16/Oct/19 ] | ||||||||
|
I want a Future to tell me when an index object in the catalog is destructed, either because it is dropped or the catalog gets recreated. Validate should continue as long as the index object is still in the catalog and has not been destructed. Like so:
There's currently no easy way to tell whether something has happened because: the recreated catalog will say the index still exists; the index could be dropped and recreated so again asking the catalog would say the index still exists. I want to know when something happened to that original IndexCatalogEntry instance. I think Eric and Geert want to take this ticket in a different direction: ensuring that the index object is safe to use across index drops and catalog recreates. That would be a lot of work, though. This idea might still have a place in that solution, or as a simple more immediate solution. | ||||||||
| Comment by Mira Carey [ 10/Oct/19 ] | ||||||||
|
Gotcha. So are you looking for a component to let you check if an entry has been dropped outside the lock? Or just to unify multiple codepaths? The main reason I ask is that, when I think of what future's are good for, I tend to think of:
And I'm trying to understand what your callers need:
| ||||||||
| Comment by Dianna Hohensee (Inactive) [ 10/Oct/19 ] | ||||||||
|
The callers of isDropped() or getCatalogGeneration() hold locks: isDropped() a collection lock; getCatalogGeneration(), most locks would work because I think restartCatalog takes a global X lock. | ||||||||
| Comment by Mira Carey [ 10/Oct/19 ] | ||||||||
|
Sorry for butting in, but I wanted to ask what sort of concurrency guarantees the isDropped() method and catalog generation number offer? The weak_lock lock() call actually bumps a refcount and safely avoids destroying an object, but I'm not sure how you'd actually use a future which told you when an object would get destroyed. I.e.
I guess because there's external locking around calling isDropped() and the like? |