Skip to content

RTreeIndex: implement CollectAllPages so spatial indexes can be freed and rebuilt #189

Closed

Description

@CaffeinatedCoder

RTreeIndex can't enumerate its pages. CollectionSecondaryIndex.CollectAllPages returns an empty list for spatial indexes, and FreeAllPages does nothing for them. As a result:

  • VACUUM RebuildIndexes skips spatial indexes (DocumentCollection.VacuumAsync, if (index.Definition.Type == IndexType.Spatial) continue;, fix(vacuum): rebuild typed-collection indexes into fresh trees (#167) #170). A rebuild would swap in a fresh tree but couldn't free the old one, so every vacuum would leak the whole old R-tree, and a failed rebuild would leak its half-built copy.
  • Dropping a spatial index or its collection leaks its pages. CollectionIndexManager.DropIndex → FreeAllPages is a no-op, and the drop-collection path in StorageEngine.Memory.cs skips IndexType.Spatial. Its comment says the pages are "reclaimed by a subsequent VACUUM pass", but as far as I can tell no vacuum pass reclaims orphaned index pages.
  • A model-declared spatial index whose backfill fails leaks its pages (CreateUnregistered doc comment, fix(index): fill model-declared indexes from existing documents (#171) #175).

Proposal:

  • Add CollectAllPages() and CollectAllPages(ulong transactionId) to RTreeIndex, as in BTreeIndex/VectorSearchIndex: walk from _rootPageId, read each node as the given transaction sees it, and follow pointer.PageId for non-leaf entries. The transaction overload matters. SplitNode allocates pages with AllocateIndexPage() outside the transaction, and they are linked only from nodes written in it, so after a rollback they can't be found from the committed tree.
  • Return it from CollectionSecondaryIndex.CollectAllPages, and use it in the drop-collection path.
  • Remove the spatial skip in VacuumAsync. Change Vacuum_RebuildIndexes_DocumentCollection_Spatial_IsKept to check that the tree is rebuilt and the old pages are freed, and add a failed-rebuild leak test for a split spatial tree.

Related: DynamicCollection.VacuumAsync with RebuildIndexes builds fresh trees for every index kind but doesn't seem to free any of the old ones.

Requested in the review of #170.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    • Status
      Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions