Skip to content

fix: Preserve tombstones during L0 compaction#43

Merged
IsaacCheng9 merged 4 commits into
mainfrom
fix-tombstone-drop-in-l0-compaction
May 21, 2026
Merged

fix: Preserve tombstones during L0 compaction#43
IsaacCheng9 merged 4 commits into
mainfrom
fix-tombstone-drop-in-l0-compaction

Conversation

@IsaacCheng9
Copy link
Copy Markdown
Owner

Summary

  • Fixed a correctness bug where compact_sstables dropped tombstones during L0→L1 compaction, letting deleted keys resurface
    • The merge gated every write on value.has_value(), so tombstones (nullopt values) were never written to the output file
    • With L1 holding overlapping files (each L0 -> L1 compaction appends a new L1, and there's no L2+ to drain into), a dropped tombstone left older L1 files free to surface the deleted key's pre-delete value on read
    • Fixed by writing all entries including tombstones, since L1 is the bottom level here; renamed the local wrote_live_entry to wrote_any_entry to match
  • Added regression test TombstoneSurvivesL0ToL1Compaction reproducing the bug via two consecutive L0 -> L1 compactions with the tombstone landing in the second
  • Updated two existing tests that pinned the buggy behaviour
    • NewSSTableDeletesKeys -> NewSSTableTombstonesShadowOlderValues: asserts the tombstone is preserved in the output (key present as a tombstone) rather than absent
    • CompactionThatDeletesAllKeysDoesNotPublishEmptyLevelOneFile -> CompactionWithOnlyTombstonesPublishesL1File: tombstone-only compactions now publish an L1 file (the tombstones are load-bearing); the engine still returns nullopt because the tombstone shadows correctly on read

Rationale

  • Preserving tombstones unconditionally means they accumulate in L1 (nothing drains them, since there's no L2+); acceptable at this project's scale
  • A production engine would add L1 -> L2 compaction with non-overlapping L2, where a tombstone can safely drop once no lower level holds the key
  • The alternative – scanning all other L1 files at compaction time to decide whether each drop is safe – is more complex and not worth it for this engine at this point

@IsaacCheng9 IsaacCheng9 merged commit d887e3c into main May 21, 2026
4 checks passed
@IsaacCheng9 IsaacCheng9 deleted the fix-tombstone-drop-in-l0-compaction branch May 21, 2026 07:49
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