~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

Deleting a ref deletes its record

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:storage/delete-refs

What changes

Deleting a ref deletes its record. A push that deletes a branch or tag writes the claim on the ref's current generation, as it always did, and deletes the ref record. It no longer updates the record to one with no target.

A new ref starts at the time. Its generation starts at the time in milliseconds (keys::first_generation), not at 1. It still goes up by one per move. The ref record gains an optional createdAt, the same as updatedAt until the ref first moves. The index uses that to recognise a ref's creation for push history. It keys each creation by the generation before it, so a ref deleted and made again has a row for each making.

Compaction drops the records older clients left. Each record with no target that last moved more than a day ago (KNOTLESS_CLAIM_GRACE_SECS) is deleted beside a claim on its generation. knotless compact and the MCP compact tool report how many as refs.

Why

A repository kept a record for every branch it ever had. chadtmiller.com/knotless holds 600 ref records, and 552 of them are deleted branches. About 50 more are added each day, one for every pull's branch. Every snapshot reads them all: a push, a fetch, the gateway's pushRefs, the runner, and every sync of knotless-serve's mirrors. Today that's 7 pages, 0.6s read side by side. At this rate it would be about 180 pages within a year.

The record was kept so a ref's generation would never repeat. That stops a claim from an earlier life of the ref from being replayed against a new one. Here is why the same guarantee holds without it:

  • A writer that read the ref before it was deleted collides with the delete's own claim. Claims are kept a day, and pushes refuse reads older than 12 hours.
  • Two racing creators collide on the create.
  • A ref made again starts at the time in milliseconds. That is past any generation an earlier ref of the name reached, since no ref moves once a millisecond. So the earlier life's claims never stand in its way.
  • For a purged legacy record, the claim compaction writes on its generation stops a writer that read the record from overwriting a ref made since.

Two alternatives were considered:

  • Fetching only the refs the feed names. That fixes only knotless-serve and leaves the records growing.
  • Reading refs from the index. That trusts Jetstream's unverified record bodies, where today a false event can only cause an extra sync.

How it was checked

Unit tests cover the index keeping each making of a ref deleted and made again, and compaction picking only deleted refs a day old, with the claim beside each delete.

compaction_keeps_young_packs_and_drops_dead_refs runs the rest against the reference PDS:

  • a new ref starts at a time-based generation, and keeps its createdAt as it moves;
  • a delete leaves no record;
  • a legacy record with no target, seeded long ago, is dropped by compact, which reports refs: 1, and the claim on its generation remains.

Coverage and the rest are left to CI.

Shortcomings

A new ref's generation leans on the writer's clock. One running far enough behind can make a ref at a generation an earlier life of it passed through, and its push off that generation is then refused until compaction drops the earlier claim. docs/design/storage.md says so.

Clients before this release still make a new ref at generation 1. Suppose one makes a branch again under a name a new client deleted less than a day earlier. Its push off generation 1 can then hit the earlier life's claim and be refused until compaction drops that claim. Once this is released, prod's KNOTLESS_CLIENT_MINIMUM should be raised to the new version.

The existing records go only as each repository is compacted. Pushes compact once past KNOTLESS_COMPACT_THRESHOLD packs, or the owner can run knotless compact.

🤖 Generated with Claude Code

Asked for changes

This pull makes a branch or tag delete remove its ref record (the claim still goes on the current generation) and starts new refs at the time in milliseconds. Compaction now drops the empty records older clients left, and the index keys each making of a ref by the generation before it. The reasoning in the body holds up, and the docs, lexicon and changeset all move with the code. I found one thing to fix.

Nothing runs the new compaction write. dead_refs is unit-tested, but both flow tests that run compact (cli.rs, git_flows.rs) report 0 deleted refs' records dropped. No test seeds a targetless ref record, so the remote.apply in the new dead.chunks(200) loop, and the refs: dead.len() / 2 count, are never run against the PDS. The body says these flows run the new writes, but they only run the zero case. The 100% coverage gate in test will likely fail on that loop, and the claim-plus-delete pair has never been applied for real. Add a flow that writes a legacy record (no target, updatedAt older than the grace) and checks that compact reports 1 and that the record is gone, with the claim present. test was still queued when I read the checks, so confirm against it.

Not blocking: first-generation uniqueness relies on clients' clocks. A client whose clock runs behind can recreate a ref at a generation an earlier life already passed through, which would collide with that life's still-kept claim. It is rare and the failure is a refused push, so I would only mention it in the docs.

crates/knotless/src/compact.rsline 99
let dead = dead_refs(&snap, &key, since);
// In pairs, each claim with the delete it guards.
for chunk in dead.chunks(200) {
remote.apply(chunk.to_vec(), &session.auth()).await?;
review.knotless.dev

This apply, and the refs count below, are not run by any test: both compact flow tests expect 0 dropped refs. Add a flow that seeds an old targetless ref record and checks it is dropped beside its claim.

Approved

This round fixes what I asked for. The new compaction_keeps_young_packs_and_drops_dead_refs flow in library.rs writes a legacy targetless ref record that is older than the grace period. It checks that compact reports refs == 1, that the record is gone, and that the claim on its generation exists. That runs the dead.chunks(200) apply loop and the refs count against the PDS. It also checks that a delete removes the ref's record, and that a new ref starts at the time in milliseconds with createdAt set. I also asked for a note on the clock-skew caveat, and storage.md now has it. I found nothing else wrong in the push, index, compaction and doc changes. The test and check runs were still queued when I read them, so confirm they pass before merging, in particular the coverage gate.

Approved

Since my last approval, this pull merged main and changed one test. In gateway.rs, the check for who pushed a deleted ref now reads the delete's claim record. The ref record is gone after a delete, so that is where the pusher shows. The change is consistent with the pull and I found nothing wrong with it. My earlier requests are still met: the compaction flow test, and the clock-skew note in storage.md. The test and check runs were still queued or running when I read them. Confirm they pass before merging, in particular the coverage gate.

Merged as d4fbc29