Skip to content

fix: 5 correctness bugs from external review (stale reads, cache epochs, silo bypass)#142

Merged
JustMaier merged 1 commit intomainfrom
fix/review-findings-v3
Apr 5, 2026
Merged

fix: 5 correctness bugs from external review (stale reads, cache epochs, silo bypass)#142
JustMaier merged 1 commit intomainfrom
fix/review-findings-v3

Conversation

@JustMaier
Copy link
Copy Markdown
Contributor

Summary

Fixes 4 HIGH + 1 MEDIUM severity issues found by GPT-5.4 external review of V3 cleanup.

  1. HIGH: ConcurrentEngine stale reads — alive_count() and reconstruct_sort_value() now read from silo when present
  2. HIGH: Cache staleness ignores alive changes — bump_field_epochs() now tracks AliveInsert/AliveRemove via "alive" epoch; cache seeding includes alive epoch
  3. HIGH: merge_bitmap_maps() bypassed silo — now calls write_dump_maps() for batch frozen write when silo present
  4. HIGH: Time bucket stale sort state — flush thread uses frozen_reconstruct_value() from silo when has_silo
  5. MEDIUM: alive_count() inconsistency — executor derives from OnceCell-cached bitmap for per-query consistency

Files changed

  • src/engine/concurrent_engine.rs — alive_count, reconstruct_sort_value, bump_field_epochs, merge_bitmap_maps
  • src/engine/executor.rs — alive_count derives from cache
  • src/engine/flush.rs — silo-based sort reconstruction for buckets
  • src/engine/query.rs — alive epoch in cache seeding

Test plan

  • cargo check --lib clean (1 warning)
  • All existing tests pass

🤖 Generated with Claude Code

…poch gaps)

Fix 1: alive_count() and reconstruct_sort_value() in ConcurrentEngine now read
from BitmapSilo when present instead of stale in-memory SlotAllocator/SortIndex.

Fix 2: bump_field_epochs() now tracks AliveInsert/AliveRemove under the
"__alive__" field key. Cache seeding in query.rs now includes the __alive__
epoch so inserts/deletes correctly invalidate cached results.

Fix 3: QueryExecutor::alive_count() now derives from alive_bitmap() via the
OnceCell, ensuring per-query consistency with the cached alive set.

Fix 4: merge_bitmap_maps() now routes writes to the silo via write_dump_maps()
when a BitmapSilo is present (skipping stale in-memory writes), and falls back
to the in-memory path only when no silo is configured.

Fix 5: Flush thread time bucket maintenance now uses frozen_reconstruct_value()
from the silo when has_silo is true, instead of reading from the in-memory
SortIndex that is not updated on the silo path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@JustMaier JustMaier merged commit 7849b58 into main Apr 5, 2026
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.

1 participant