Skip to content

bugfix: clear spilled hybrid update state after emit batches#1147

Merged
yokofly merged 2 commits intodevelopfrom
clear-spilled-hybrid-state
Mar 27, 2026
Merged

bugfix: clear spilled hybrid update state after emit batches#1147
yokofly merged 2 commits intodevelopfrom
clear-spilled-hybrid-state

Conversation

@yokofly
Copy link
Copy Markdown
Collaborator

@yokofly yokofly commented Mar 27, 2026

Summary

  • HybridHashTable::clear() and HybridKeyList::clear() both had the same stale-persistent-data bug: when cleanup_on_disk_data=true the cf_handler shared_ptr was reset without calling destroyPersistentPart(), leaving stale keys in the shared RocksDB column family that would reappear on the next initRocks() call (e.g. after checkpoint recovery).
  • Fix both clear() methods by adding an else { destroyPersistentPart(); } branch so the column family is dropped before the handler is released.
  • Add symmetric {} braces to both if/else branches in both methods.
  • Add TEST(HybridKeyList, ClearRemovesSharedPersistentData) regression test mirroring the HybridHashTable one.
  • Update the SQL regression test (99248) to exercise the pause/resume checkpoint-recovery path that most reliably triggers the re-emission of unchanged groups.

Root cause

After clear(), cf_handler.reset() dropped the shared_ptr but the RocksDB column family entry remained live in RocksDB::cf_handles. The next getOrCreateColumnFamilyHandler() call found the existing CF with all old data, causing forEachKey() / forEach() to iterate stale spilled keys alongside current ones and re-emit unchanged aggregation groups.

When ManyAggregatedData is destroyed, its 'rocks' member (vector of
shared_ptr<RocksDB>) is destroyed before 'variants'. This causes
RocksDB::shutdown() to free the underlying rocksdb::DB. The subsequent
destruction of variants triggers RocksDBColumnFamilyHandler::destroy(),
which accesses db->DefaultColumnFamily() through a dangling raw pointer.

Fix by locking the weak_ptr<RocksDB> before any raw db pointer access.
If the RocksDB is already gone or shut down, return early since
RocksDB::shutdown() already cleaned up all column family handles.
@yokofly yokofly merged commit c2d595c into develop Mar 27, 2026
3 of 4 checks passed
@yokofly yokofly deleted the clear-spilled-hybrid-state branch March 27, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants