Skip to content

Disk reclaim can delete an object a later write put at the same key #56

Description

@anoop-narang

A store key is (entry id, identity) (entry_id_to_key), and identities are reused — FileIdPool::acquire searches the free queue by path and restores a re-opened file's previous identity so its cached entries stay readable.

reclaim_orphaned_disk asks the index whether a live record still names the object before deleting it, which closes the common case. It does not close all of it: a put that has landed while its index record is not yet installed is invisible to that check.

write_batch_to_disk completes store.put before the caller's separate try_insert installs the record, so that window is real rather than theoretical.

t4 applies puts and tombstones by LSN (store.rs, apply_put / apply_remove, with an explicit Rejected arm for "a concurrent put or remove with a later LSN beat us to the index"). A reclaim's remove issued after the fresh put therefore carries the later LSN and wins.

Sequence

1. reclaim checks holds_disk_entry   -> false (record not installed yet)
2. fresh writer's put lands           LSN N
3. reclaim's remove lands             LSN N+1   -> deletes the fresh object
4. writer's try_insert installs a record naming a deleted object
5. read -> NotFound, and the reservation stays charged

Fix direction

A per-write generation in the store key, so a reclaim names the write it decided against rather than whichever write currently holds the writer's key. The generation can live in the index — in the Slot beside identity, or in the DiskArrow / DiskLiquid variants — so a re-opened file's cached entries stay readable.

Consequence worth deciding up front: with a unique generation per write, a disk write never lands on the object it displaces, so DiskResidue::superseded and the in-place-overwrite case become unreachable and should be removed.

Cost: it touches every read, write and delete path, and get_checked has to hand the generation back.

What would close this

A deterministic test that pauses a fresh writer after store.put and before its index insert, runs stale reclamation, and asserts the eventual disk record is still readable with consistent budget accounting.

Not a pre-existing hole

The reclamation layer is recent. The pin this fork ran before it had no reclaim_orphaned_disk, DiskResidue, settle or remove_checked at all, so this race arrived with that layer rather than being inherited and narrowed.

Reported by review on an earlier PR; recorded here so it is not re-reported as new and not carried in CLAUDE.md, which should not hold facts that expire.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions