[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:

{
    ....
    if (future.isSet()) {
        uassert(future.getValue());   <-- value being an error Status.
    }
    ....
}

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:

  1. A way to return results to blocking callers without taking larger locks and to handle lifetime for returned results
  2. Support for continuations and async programming

And I'm trying to understand what your callers need:

if (entry.isDropped()) {
    // this makes a bunch of sense to have as a future
} else {
    // I don't know what you can do with an unready future here
}

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.

fut = getFutureForWhenIndexCatalogEntryIsDestroy(entry);
 
if (fut.isReady()) {
    return fut.getNoThrow();
} else {
    // why aren't we worried about a race with fulfilling the future here?
    doStuff(entry)
}

I guess because there's external locking around calling isDropped() and the like?

Generated at Thu Feb 08 05:04:33 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.