Harden CollectionCatalog::establishConsistentCollection against misuse

XMLWordPrintableJSON

    • Type: Improvement
    • Resolution: Unresolved
    • Priority: Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • None
    • Catalog and Routing
    • 200
    • 2
    • 🟦 Shard Catalog
    • None
    • None
    • None
    • None
    • None
    • None

      CollectionCatalog::establishConsistentCollection contract is confusing and easy to misuse.

      1. The method's header talks about "Establish a collection instance consistent with the opened storage snapshot.", but doesn't have any invariants in place to verify the storage snapshot is already open.
        1. e.g. the mdb_catalog reads can end up opening the storage snapshot anyways, and apparently there's already code working around this issue.
      2. readTimestamp == boost::none means use latest collection instance, so it is not necessarily consistent with the snapshot. Moreover, even if we pass boost::none, the first thing the method does is check _findInstantiatedCollectionByNamespaceOrUUID, in principle there could be an instance in OpenedCollections that is not the latest, and return that.
      3. The caller can pass an arbitrary timestamp, even if that is different from what shard_role_details::getRecoveryUnit(opCtx)->getPointInTimeReadTimestamp() would return based on the ReadSource.
        1. We've recently seen a case where establishConsistentCollection was used improperly by passing readTimestamp==boost::none with an already open snapshot and a ReadSource different from kNoTimestamp.

            Assignee:
            Unassigned
            Reporter:
            Yujin Kang Park
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated: