keyvalue: prefix deletion watermark - #759
Open
cheggaaa wants to merge 4 commits into
Open
Conversation
A synced signed row (StoreKeyInner.deletePrefix) whose apply physically drops every row under the prefix older than it — document and ldiff element — and rejects such rows arriving later, so restores from old snapshots cannot resurrect them. The watermark row is the only retained state. Owner-only, on both the local API (Storage.DeletePrefix) and the receive path. Validation now also derives KeyPeerId from the signed payload, closing an impersonation gap: an unchecked id let any writer occupy another peer's row. SYN-145
Coverage provided by https://github.com/seriousben/go-patch-cover-action |
- applyWatermark filters on the row's key, not its id: an id can match a
prefix past its key's end (key+"-"+peerId), which deleted rows of a
shorter key and diverged replicas from the covered() rejection rule
- failed-tx diff undo restores a per-id pre-call snapshot instead of
replaying prior/added/removed lists, which advertised phantom heads
when a row was written and watermark-dropped in the same batch
- inner Set returns SetResult (applied values + dropped ids): SetRaw now
broadcasts and indexes only applied rows — indexing store-rejected rows
resurrected deleted data downstream
- optional DeletionAwareIndexer.RemoveIndex mirrors watermark drops into
the app index
- local Set/DeletePrefix under a newer covering watermark fail with
ErrCoveredByWatermark instead of silently succeeding while every
replica discards the row
- DeletePrefix("") rejected
- syncWithPeer tolerates a row deleted between CompareDiff and its load
- IteratePrefix and applyWatermark share one id-prefix iterator; the
break condition is HasPrefix (Contains could over-include mid-id
matches past the range)
SYN-145
The deletion operation is its own message inside the signed StoreKeyInner (inner.delete != nil) instead of a bare string field: the schema states what the field does, the operation type cannot be forged (an unsigned outer discriminator could be), and future deletion options have a typed home. Wire mechanics unchanged — the watermark stays an ordinary synced row, which anti-entropy and p2p sync require. SYN-145
Stale unsynced rows drop on both writer and hub once the watermark arrives; rows written after the deletion win by LWW and propagate until a re-issued delete removes them everywhere; unsynced rows outside the prefix sync normally. SYN-145
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds physical deletion to the keyvalue store. Needed by SYN-104 (prune
read/frontiers of removed spaces): today rows are (key, peerId) LWW synced by ldiff, so nothing can ever be removed — a local delete resurrects on the next diff round, and dead-space rows accumulate on every device and node forever.Design
A deletion is a normal synced row whose signed
StoreKeyInnercarries a typedStoreDeletePrefixoperation (inner.delete) — the watermark. It must be an ordinary row: the stream frame type is frozen (changing it breaks normal sync with old peers), the marker has to participate in ldiff anti-entropy to reach every replica including restored devices, and the operation discriminator must live under the signatures so it cannot be forged. Applying it:Surface:
Storage.DeletePrefix(ctx, prefix). Owner-only on both the local API and the receive path (a watermark removes other peers' rows).GetAll/Iteratehide watermark rows. InnerSetreturns aSetResult(applied values + dropped ids):SetRawbroadcasts and indexes only applied rows, and the optionalDeletionAwareIndexer.RemoveIndexmirrors drops into the app index. Local writes under a newer covering watermark fail withErrCoveredByWatermarkinstead of silently succeeding while every replica discards the row.Validation change bundled:
KeyPeerIdis now derived from the signed payload (key + "-" + inner.peer). Previously it was unchecked (the TODO inelement.go), letting any writer occupy or overwrite another peer's row with a forged id.Rollout
Old peers store a watermark as an inert empty-valued row and keep their data. Plan: ship the support to the whole fleet first; enable actual
DeletePrefixcalls only after adoption, so no watermark exists while old versions run.Tests
SYN-145
🤖 Generated with Claude Code