From b8d15c4564f6e3b2c7f737dd1b808879757802a1 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Mon, 28 Sep 2026 21:49:27 +0300 Subject: [PATCH 1/8] fix(bucketindex): scope block identity to the tenure and fence commits on it Blocks are (term, n) under the claim's etcd revision (index format v7), entries carry their writing term, and a clustered engine refuses a commit it can no longer make as the shard's current tenure. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KcLq1G8KpWBvM53JaQb1tS --- ARCHITECTURE.md | 12 +- backend/ARCH.md | 54 +++- backend/bucketindex/alloc_test.go | 89 ++++- backend/bucketindex/bench_test.go | 81 +++++ backend/bucketindex/block.go | 305 ++++++++++++------ backend/bucketindex/block_test.go | 119 ++++--- backend/bucketindex/blockset_test.go | 36 ++- backend/bucketindex/bucketindex.go | 156 +++++++-- backend/bucketindex/bucketindex_test.go | 15 +- backend/bucketindex/format_test.go | 174 +++++++--- backend/bucketindex/generation.go | 7 +- backend/bucketindex/groupsuccessor_test.go | 8 +- backend/bucketindex/hole_test.go | 16 +- backend/bucketindex/lineage.go | 20 +- backend/bucketindex/lineage_test.go | 2 +- backend/bucketindex/plan.go | 186 +++++++++++ backend/bucketindex/tenure.go | 29 ++ backend/bucketindex/tenure_test.go | 78 +++++ .../fuzz/FuzzRoundTrip/5b43b470a18e2e78 | 1 + backend/bucketindex/want.go | 5 +- backend/bucketindex/want_test.go | 18 +- cluster/ARCH.md | 7 + cluster/partsync/deferredsync_test.go | 2 +- cluster/partsync/omitted_test.go | 6 +- cluster/partsync/repair_test.go | 24 +- cluster/partsync/repairlisting_test.go | 4 +- cluster/partsync/repairselect_test.go | 14 +- cluster/partsync/wants_internal_test.go | 10 +- cluster_recovered_seams_test.go | 45 ++- cluster_tenure_test.go | 4 +- engine/ARCH.md | 22 +- engine/blockid.go | 171 +--------- engine/engine.go | 8 +- engine/export_test.go | 2 +- engine/index.go | 101 ++++-- engine/merge.go | 5 +- engine/part.go | 4 +- engine/repair.go | 4 +- engine/splitlineage_test.go | 4 +- internal/enginetest/blockid.go | 26 +- internal/enginetest/enginetest.go | 4 + internal/enginetest/heldpart.go | 2 +- internal/enginetest/hole.go | 20 +- internal/enginetest/repair.go | 2 +- internal/enginetest/repairfetch.go | 34 +- internal/enginetest/splitlineage.go | 2 +- internal/enginetest/tenure.go | 19 +- internal/enginetest/tenurefence.go | 161 +++++++++ internal/enginetest/wants.go | 8 +- internal/repair/engine_test.go | 2 +- internal/repair/group_test.go | 6 +- internal/repair/repair.go | 20 +- internal/repair/repair_test.go | 18 +- recordengine/ARCH.md | 6 +- recordengine/blockid.go | 171 +--------- recordengine/engine.go | 8 +- recordengine/export_test.go | 2 +- recordengine/index.go | 96 ++++-- recordengine/merge.go | 5 +- recordengine/mergewriter_internal_test.go | 12 +- recordengine/part.go | 4 +- recordengine/repair.go | 4 +- recordengine/splitlineage_test.go | 4 +- 63 files changed, 1680 insertions(+), 804 deletions(-) create mode 100644 backend/bucketindex/bench_test.go create mode 100644 backend/bucketindex/plan.go create mode 100644 backend/bucketindex/tenure.go create mode 100644 backend/bucketindex/tenure_test.go create mode 100644 internal/enginetest/tenurefence.go diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9748a80f..77a612a4 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -197,6 +197,12 @@ to fail over to: a read overlapping a want fails. stamping ownership terms, while still serving reads and still applying replicated writes. Gating on the lease deadline rather than on etcd reachability is what keeps a blip free and a partition safe (`cluster/ARCH.md`, *Lease fencing*). +- **A clustered writer commits only as the shard's current tenure.** Every index commit first + re-checks that the writer still holds its claim, under the term the operation began with, and + that the index it builds on was not written by a later term (`bucketindex.CheckTenure`). Part + identity is scoped the same way: a block is a `(term, n)` pair, so two tenures that never saw each + other's index cannot allocate the same identity. What the fence leaves possible is duplicate rows, + never a collision (`backend/ARCH.md`, *Part identity*). - **The bucket index commits last, conditionally.** It is what makes a part durably visible — and in `recordengine` it also carries the flush watermark (the WAL replay floor, one slot per writer since the watermark is per node and the index is shared) — so its write is the @@ -253,9 +259,9 @@ to fail over to: a read overlapping a want fails. reads as unknown, never raw) / marks (`OTMK`) / column object framing (the trailer-dictionary bytes column; the leading layout still read) and key layout, the attribute hash+binary encoding (the SeriesID pre-image), symbol table (`OTSY`), the bucket index (format - v6: entries with their block sets and split claims, tombstones, wants, per-writer flush - watermarks and the block high-water mark — readable back to v1, not writable by older - nodes), WAL record framing + v7: entries with their term-scoped block sets, split claims and writing term, tombstones, wants, + per-writer flush watermarks and the block high-water mark — readable back to v1, not writable by + older nodes), WAL record framing (additive record types), record-key footer (`OTKY`), the metric part column layout (`[series:int128, ts:int64, value:float64]` + optional `sf:float64`), and the profile symbol-store sidecar (`OTSP`, version 2: a compressed body with its raw length; version 1 still diff --git a/backend/ARCH.md b/backend/ARCH.md index c03da078..c33bb08d 100644 --- a/backend/ARCH.md +++ b/backend/ARCH.md @@ -165,8 +165,11 @@ a demonstration. to commit at all and wraps `ErrFenced`: a version is only safe to commit against together with the part set read with it (`engine/ARCH.md`). - **Part identity is a block *set* plus a level** (`bucketindex.Interval`, `Entry.Level`, format - v6). A flush writes `{n}` at level 0; a merge writes the union of what its inputs covered, above - them. `Entry.Supersedes` is then decidable from identity alone — no index diff, no bookkeeping, + v6). A flush writes `{b}` at level 0; a merge writes the union of what its inputs covered, above + them. **A block is a `(term, n)` pair** (`bucketindex.Block`, format v7): the ownership term of the + tenure that allocated it and a number within that term. Blocks order by term, then number, and a + merge across tenures holds each tenure's blocks with the rest of the lower term's number space as + an ordinary gap. `Entry.Supersedes` is then decidable from identity alone — no index diff, no bookkeeping, no data read — which is what lets a repair terminate: by the time a want is serviced the data may exist only inside a merged successor, and `Index.Satisfying` accepts that successor as discharging the want. @@ -203,9 +206,17 @@ a demonstration. short of reading the rows says how. A part covering only some of a group's claim therefore overlaps every member while replacing none; no single-part rule resolves it, and repair refuses to publish one beside the other (`engine/ARCH.md`, "Repair"). - **Allocation is the shard owner's alone**: `Index.NextBlock` is one above `AllocatedBlocks`, the - persisted high-water mark, and above every block the live entries, wants and group runs still - name. The mark is what keeps identity unique over a shard's *whole life* rather than over its + **Allocation is the shard owner's alone, and scoped to its tenure**: `Index.NextBlock(term)` is one + above `AllocatedBlocks`, the persisted high-water mark, and above every block the live entries, + wants and group runs still name — within `term`, starting at `(term, 1)` for a tenure the index has + not seen. Scoping is what makes identity unique across writers that never see each other's index: + a displaced owner and its successor, or two replicas whose lineages diverged, each allocate only + under their own claim's etcd create revision, which no other claim shares. The obvious alternative, + one number space with the loser of a CAS re-allocating above the winner, holds only while both + writers commit to one object; over diverged copies it hands two unrelated parts one identity, and + every identity relation then treats them as one part. A term below the index's highest one + continues that term's sequence instead of restarting its own; only a writer with no cluster (term + 0, the prefix's only writer) reaches that, since a clustered one is fenced first (below). The mark is what keeps identity unique over a shard's *whole life* rather than over its current contents: the live set shrinks — retention can empty a shard outright, leaving tombstones that carry no blocks — and numbering derived from it would rewind and hand a new part the identity an expired one held, at which point a stale peer's old part satisfies a want for the new one and @@ -213,8 +224,25 @@ a demonstration. that adds the part; a writer rebasing on a rival's index takes the maximum of the two marks — see `engine/ARCH.md`, "Block identity is allocated by the commit that publishes the part". No etcd, no round trip on the flush path, and it works with the cluster layer absent. Two owners racing a - handoff resolve through that CAS: one commit lands, the loser wraps `ErrConflict`, re-reads, and - re-allocates above the winner. + handoff over one index resolve through that CAS: one commit lands, the loser wraps `ErrConflict`, + re-reads, and re-allocates above the winner. + **One identity written by two tenures resolves to the later one** (`Entry.Term`, format v7). The + fence below leaves one way for it to arise: two owners merge the same inputs either side of a + handoff, the earlier one committing while its claim is still provable. Both outputs cover the same + blocks at the same level and hold the same rows, so `Lineage.Subsumes` — and through it + `Supersedes`, `Satisfying`, `Revokes` and `Subsumed` — lets the higher `Term` replace the lower. +- **A clustered writer commits only as the shard's current tenure** (`CheckTenure`, `ErrSuperseded`). + Before each CAS attempt the engine checks the term it holds now against the term its operation + began under and the term of the index it builds on. It refuses when it holds no claim (a lapsed or + fenced lease reads as term 0), when its tenure ended and restarted mid-operation, and when a + rebase shows a later tenure wrote the index — a rebase never adopts a higher term, so a displaced + writer never commits under its successor's. A refused commit leaves its output unreferenced, for + the orphan sweep. It is a check before a CAS, not inside one: a commit that passes can still land + after the claim lapses, and what bounds that window is the lease fence margin, since a successor + can only acquire once the lease has expired. What the window admits is duplicate rows — an old + owner's last flush, re-flushed by the new owner from its replica head, or overlapping merges — + never a collision, a stuck want or a false hole. A writer with no cluster has no tenure and is not + fenced. - **`Entries → Removed | Wanted`** is the invariant the repair path enforces: a part leaves `Entries` only into a tombstone (`Removal`, a deliberate deletion) or into a `Want` (an obligation to fetch it back). Conflating the two would make "am I repaired?" unanswerable, which @@ -262,13 +290,17 @@ a demonstration. is a fact, not a level: a per-node gauge that a restart or a successful repair clears erases the only record that a range of a shard was ever acknowledged as gone. It follows ClickHouse's `/lost_part_count`. -- **Format v6 is a hard read break.** `Decode` rejects any version above the one it knows, so - reading is backward compatible (v1–v5 still decode, unset fields ordering below everything a - writer produces) but **writing is not**: a node on pre-v6 code fails on the first v6 index it +- **Format v7 is a hard read break.** `Decode` rejects any version above the one it knows, so + reading is backward compatible (v1–v6 still decode, unset fields ordering below everything a + writer produces) but **writing is not**: a node on pre-v7 code fails on the first v7 index it reads. Every node that reads a given index must be upgraded together. The path differs by deployment and both matter — on a shared backend every node reads the same index object, and in `PrivateBackend` mode `cluster/partsync` reads its peers' indexes. - Two things migrate. A **v5 interval is a hull**, and what it actually covered is unknowable once + **A v6 index is term 0 throughout**: its blocks, its entries' and wants' writing terms and its + high-water mark. A clustered tenure then allocates from `(term, 1)` above all of it, and a writer + with no cluster continues term 0's numbering where v6 left it. An interval's lowest block number + leads its encoding, so a valid set never starts with the zero byte an unset one is written as. + Two things migrate from v5. A **v5 interval is a hull**, and what it actually covered is unknowable once decoded, so it is read as the contiguous set of its bounds — the meaning it had when it was written. Such an entry can still claim a block no part held, exactly as it did under v5; a merge rewriting it produces an exact set, so the defect drains as the shard compacts rather than being diff --git a/backend/bucketindex/alloc_test.go b/backend/bucketindex/alloc_test.go index 731cdc0f..84f37003 100644 --- a/backend/bucketindex/alloc_test.go +++ b/backend/bucketindex/alloc_test.go @@ -24,7 +24,7 @@ func TestBlockAllocationResolvesThroughCAS(t *testing.T) { const key = "default/metrics/" + bucketindex.Object base := &bucketindex.Index{} - base.Add(bucketindex.Entry{Prefix: "p1", Blocks: bucketindex.Interval{Min: 1, Max: 1}}) + base.Add(bucketindex.Entry{Prefix: "p1", Blocks: bucketindex.Range(0, 1, 1)}) _, err := base.Save(ctx, be, key, backend.VersionAbsent) require.NoError(t, err) @@ -33,11 +33,11 @@ func TestBlockAllocationResolvesThroughCAS(t *testing.T) { newOwner, newVersion, err := bucketindex.LoadVersioned(ctx, be, key) require.NoError(t, err) - assert.EqualValues(t, 2, old.NextBlock()) - assert.EqualValues(t, 2, newOwner.NextBlock(), "both owners allocate the same block") + assert.Equal(t, bucketindex.Block{N: 2}, old.NextBlock(0)) + assert.Equal(t, bucketindex.Block{N: 2}, newOwner.NextBlock(0), "both owners allocate the same block") - old.Add(bucketindex.Entry{Prefix: "p2-old", Blocks: bucketindex.Interval{Min: 2, Max: 2}}) - newOwner.Add(bucketindex.Entry{Prefix: "p2-new", Blocks: bucketindex.Interval{Min: 2, Max: 2}}) + old.Add(bucketindex.Entry{Prefix: "p2-old", Blocks: bucketindex.Range(0, 2, 2)}) + newOwner.Add(bucketindex.Entry{Prefix: "p2-new", Blocks: bucketindex.Range(0, 2, 2)}) _, err = old.Save(ctx, be, key, oldVersion) require.NoError(t, err) @@ -48,23 +48,23 @@ func TestBlockAllocationResolvesThroughCAS(t *testing.T) { // The retry: reload, re-allocate, commit. The winner's block stands. retry, retryVersion, err := bucketindex.LoadVersioned(ctx, be, key) require.NoError(t, err) - assert.EqualValues(t, 3, retry.NextBlock(), "allocation moves above the block that landed") + assert.Equal(t, bucketindex.Block{N: 3}, retry.NextBlock(0), "allocation moves above the block that landed") - retry.Add(bucketindex.Entry{Prefix: "p2-new", Blocks: bucketindex.Interval{Min: 3, Max: 3}}) + retry.Add(bucketindex.Entry{Prefix: "p2-new", Blocks: bucketindex.Range(0, 3, 3)}) _, err = retry.Save(ctx, be, key, retryVersion) require.NoError(t, err) got, err := bucketindex.Load(ctx, be, key) require.NoError(t, err) - blocks := map[uint64]string{} + blocks := map[bucketindex.Block]string{} for _, e := range got.Entries { _, dup := blocks[e.Blocks.Min] - require.Falsef(t, dup, "block %d claimed twice", e.Blocks.Min) + require.Falsef(t, dup, "block %v claimed twice", e.Blocks.Min) blocks[e.Blocks.Min] = e.Prefix } - assert.Equal(t, map[uint64]string{1: "p1", 2: "p2-old", 3: "p2-new"}, blocks) + assert.Equal(t, map[bucketindex.Block]string{{N: 1}: "p1", {N: 2}: "p2-old", {N: 3}: "p2-new"}, blocks) } // TestMergeAllocatesSpanningInterval walks the identity a merge produces: the successor covers @@ -74,13 +74,10 @@ func TestMergeAllocatesSpanningInterval(t *testing.T) { ix := &bucketindex.Index{} for i := range 3 { - n := ix.NextBlock() - ix.Add(bucketindex.Entry{ - Prefix: string(rune('a' + i)), - Blocks: bucketindex.Interval{Min: n, Max: n}, - }) + b := ix.NextBlock(0) + ix.Add(bucketindex.Entry{Prefix: string(rune('a' + i)), Blocks: bucketindex.Single(b)}) } - require.EqualValues(t, 4, ix.NextBlock()) + require.Equal(t, bucketindex.Block{N: 4}, ix.NextBlock(0)) inputs := slices.Clone(ix.Entries) merged := bucketindex.Entry{ @@ -97,5 +94,63 @@ func TestMergeAllocatesSpanningInterval(t *testing.T) { } ix.Add(merged) - assert.EqualValues(t, 4, ix.NextBlock(), "a merge consumes blocks, it does not allocate new ones") + assert.Equal(t, bucketindex.Block{N: 4}, ix.NextBlock(0), "a merge consumes blocks, it does not allocate new ones") +} + +// TestTenuresAllocateDisjointBlocks is the block collision of #725: two tenures that never see each +// other's index — a displaced owner and its successor over divergent copies — both allocate the +// next block. Each allocates under its own term, so the identities cannot collide and no identity +// relation takes one part for the other. +func TestTenuresAllocateDisjointBlocks(t *testing.T) { + t.Parallel() + + base := &bucketindex.Index{AllocatedBlocks: bucketindex.Block{Term: 7, N: 4}} + base.Add(bucketindex.Entry{Prefix: "p4", Blocks: bucketindex.TermBlocks(7, 4), Term: 7}) + + displaced, successor := base.NextBlock(7), base.NextBlock(9) + assert.Equal(t, bucketindex.Block{Term: 7, N: 5}, displaced) + assert.Equal(t, bucketindex.Block{Term: 9, N: 1}, successor, "a new tenure starts its own sequence") + + a := bucketindex.Entry{Prefix: "a", Blocks: bucketindex.Single(displaced), Term: 7} + b := bucketindex.Entry{Prefix: "b", Blocks: bucketindex.Single(successor), Term: 9} + assert.False(t, bucketindex.LineageOf(nil).Overlaps(a, b), "the two flushes share no identity") + + merged := bucketindex.Entry{Prefix: "m", Blocks: a.Blocks.Union(b.Blocks), Level: 1, Term: 9} + assert.True(t, merged.Supersedes(a)) + assert.True(t, merged.Supersedes(b)) + assert.EqualValues(t, 2, merged.Blocks.Len(), "a merge across tenures holds each tenure's block") +} + +// TestNextBlockBelowTheIndexTerm pins the writer with no cluster over an index a clustered tenure +// wrote: it continues that tenure's sequence rather than restarting term 0, whose earlier blocks the +// index may no longer name. +func TestNextBlockBelowTheIndexTerm(t *testing.T) { + t.Parallel() + + ix := &bucketindex.Index{AllocatedBlocks: bucketindex.Block{Term: 5, N: 3}} + assert.Equal(t, bucketindex.Block{Term: 5, N: 4}, ix.NextBlock(0)) + assert.Equal(t, bucketindex.Block{Term: 6, N: 1}, ix.NextBlock(6)) + + ix.Add(bucketindex.Entry{Prefix: "p", Blocks: bucketindex.TermBlocks(5, 9)}) + assert.Equal(t, bucketindex.Block{Term: 5, N: 10}, ix.NextBlock(5), "live blocks above the mark still count") +} + +// TestIdenticalIdentityResolvesToTheLaterTenure is two owners that merged the same inputs either +// side of a handoff: one identity, two parts, and the later tenure's supersedes the earlier's. +func TestIdenticalIdentityResolvesToTheLaterTenure(t *testing.T) { + t.Parallel() + + blocks := bucketindex.TermBlocks(3, 1, 2) + earlier := bucketindex.Entry{Prefix: "a", Blocks: blocks, Level: 1, Term: 3} + later := bucketindex.Entry{Prefix: "b", Blocks: blocks, Level: 1, Term: 4} + + assert.True(t, later.Supersedes(earlier)) + assert.False(t, earlier.Supersedes(later)) + assert.False(t, later.Supersedes(later), "a part does not supersede itself") + assert.Equal(t, map[string]struct{}{"a": {}}, + bucketindex.Subsumed([]bucketindex.Entry{earlier}, []bucketindex.Entry{later})) + + grouped := later + grouped.Claim = bucketindex.Claim{Blocks: bucketindex.Blocks(1), Group: bucketindex.Blocks(2)} + assert.False(t, grouped.Supersedes(earlier), "a different claim is a different identity") } diff --git a/backend/bucketindex/bench_test.go b/backend/bucketindex/bench_test.go new file mode 100644 index 00000000..543b30f7 --- /dev/null +++ b/backend/bucketindex/bench_test.go @@ -0,0 +1,81 @@ +package bucketindex_test + +import ( + "fmt" + "testing" + + "github.com/oteldb/storage/backend/bucketindex" +) + +// benchIndex is a shard's index at a realistic live part count: mostly merged parts over contiguous +// runs, a few flushes, and some tombstones. +func benchIndex() *bucketindex.Index { + ix := &bucketindex.Index{Generation: bucketindex.Generation{Term: 1 << 20, Counter: 1 << 10}} + + next := uint64(1) + for i := range 100 { + width := uint64(1) + if i%4 != 0 { + width = 16 + } + + nums := make([]uint64, 0, width) + for range width { + nums = append(nums, next) + next++ + } + + ix.Add(bucketindex.Entry{ + Prefix: fmt.Sprintf("default/metrics/01M3MM88TN9KYPAV6QS8F%05d", i), + MinTime: int64(i) << 30, MaxTime: int64(i+1) << 30, + Blocks: bucketindex.Blocks(nums...), Level: uint32(i % 3), + }) + } + + for i := range 512 { + ix.Tombstone(bucketindex.Removal{Prefix: fmt.Sprintf("default/metrics/dead%05d", i)}) + } + + return ix +} + +func BenchmarkIndexEncode(b *testing.B) { + ix := benchIndex() + buf := ix.AppendBinary(nil) + + b.SetBytes(int64(len(buf))) + b.ReportAllocs() + + for b.Loop() { + buf = ix.AppendBinary(buf[:0]) + } +} + +func BenchmarkIndexDecode(b *testing.B) { + data := benchIndex().AppendBinary(nil) + + b.SetBytes(int64(len(data))) + b.ReportAllocs() + + for b.Loop() { + if _, err := bucketindex.Decode(data); err != nil { + b.Fatal(err) + } + } +} + +// BenchmarkTrimWants is the identity relation a commit runs per want over the live set. +func BenchmarkTrimWants(b *testing.B) { + ix := benchIndex() + + wants := make([]bucketindex.Want, 0, 16) + for i := range 16 { + wants = append(wants, bucketindex.Want{Prefix: fmt.Sprintf("lost%02d", i), Blocks: bucketindex.Blocks(uint64(2000 + i))}) + } + + b.ReportAllocs() + + for b.Loop() { + bucketindex.TrimWants(append([]bucketindex.Want(nil), wants...), ix.Entries) + } +} diff --git a/backend/bucketindex/block.go b/backend/bucketindex/block.go index e4c556be..8712a864 100644 --- a/backend/bucketindex/block.go +++ b/backend/bucketindex/block.go @@ -1,73 +1,154 @@ package bucketindex -import "slices" +import ( + "cmp" + "math" + "slices" +) + +// Block is one allocated block id: the ownership term that allocated it and its number within that +// term. A tenure allocates only under its own term, so two tenures of a shard — a displaced owner and +// its successor, or two replicas whose lineages diverged — never hand out the same id, with no +// coordination beyond the claim that already orders them. Term 0 is the writer with no cluster, and +// every block an index written before format v7 names. +// +// Blocks order by term, then number. Numbers start at 1 within each term, so {term, 0} is never a +// block, only the point below a term's first one. +type Block struct { + Term uint64 + N uint64 +} + +// Compare orders two blocks, as [cmp.Compare] does. +func (b Block) Compare(o Block) int { + switch { + case b.Term != o.Term: + return cmp.Compare(b.Term, o.Term) + default: + return cmp.Compare(b.N, o.N) + } +} + +// next is the successor in block order. A number space that ends wraps into the next term. +func (b Block) next() Block { + if b.N == math.MaxUint64 { + return Block{Term: b.Term + 1} + } + + return Block{Term: b.Term, N: b.N + 1} +} + +// prev is the predecessor in block order, the inverse of [Block.next]. +func (b Block) prev() Block { + if b.N == 0 { + return Block{Term: b.Term - 1, N: math.MaxUint64} + } + + return Block{Term: b.Term, N: b.N - 1} +} + +func (b Block) less(o Block) bool { return b.Compare(o) < 0 } -// Interval is the exact set of block numbers a part covers, the identity it carries alongside its -// prefix. A flush writes {n}; a merge writes the union of what its inputs covered. +func minBlock(a, b Block) Block { + if a.less(b) { + return a + } + + return b +} + +// MaxBlock is the later of a and b in block order. +func MaxBlock(a, b Block) Block { + if a.less(b) { + return b + } + + return a +} + +// Interval is the exact set of blocks a part covers, the identity it carries alongside its prefix. +// A flush writes {b}; a merge writes the union of what its inputs covered. // -// The set is stored as its bounds plus the runs of numbers it does *not* hold, because a part's +// The set is stored as its bounds plus the runs of blocks it does *not* hold, because a part's // blocks are contiguous in every ordinary case and the gap list is then empty. It is a set and not // a hull because supersession means "holds these rows": merging the parts on either side of a lost // one yields {1,3}, and a hull would claim block 2 — a want discharged by a part holding none of -// its data. +// its data. A merge across tenures holds blocks of several terms, and the space between them is a +// gap like any other. // -// Block numbers start at 1, so the zero value is *unset* rather than a range covering block 0. -// That distinction is what keeps a part written before format v5 — which carries no interval — -// from accidentally containing, or being contained by, anything: see [Interval.Valid]. +// The zero value is *unset* rather than a range covering block {0, 0}. That distinction is what +// keeps a part written before format v5 — which carries no interval — from accidentally containing, +// or being contained by, anything: see [Interval.Valid]. type Interval struct { - Min uint64 - Max uint64 + Min Block + Max Block // Gaps are the runs inside (Min, Max) the set does not cover, in ascending order, disjoint and // non-adjacent. The canonical form is the only valid one: two encodings of one set would break // both equality and the encode∘decode identity. Gaps []Gap } -// Gap is a run of block numbers an [Interval] skips: the numbers from Min to Max inclusive that -// lie inside the interval's bounds but that no part in it holds. +// Gap is a run of blocks an [Interval] skips: the blocks from Min to Max inclusive that lie inside +// the interval's bounds but that no part in it holds. type Gap struct { - Min uint64 - Max uint64 + Min Block + Max Block } -// Blocks builds the interval covering exactly the given block numbers, in any order and with -// duplicates. It returns the unset interval for an empty list or one naming block 0. -func Blocks(nums ...uint64) Interval { +// Blocks builds the term-0 interval covering exactly the given block numbers: see [TermBlocks]. +func Blocks(nums ...uint64) Interval { return TermBlocks(0, nums...) } + +// TermBlocks builds the interval covering exactly the given block numbers of term, in any order and +// with duplicates. It returns the unset interval for an empty list or one naming block 0. +func TermBlocks(term uint64, nums ...uint64) Interval { runs := make([]Gap, 0, len(nums)) for _, n := range nums { if n == 0 { return Interval{} } - runs = append(runs, Gap{Min: n, Max: n}) + b := Block{Term: term, N: n} + runs = append(runs, Gap{Min: b, Max: b}) } return fromRuns(runs) } -// Valid reports whether the interval names a real set of blocks: numbering starts at 1, the bounds -// are ordered, and the gap list is canonical. Everything else — the zero value of a pre-v5 entry, -// and any inversion or denormalization a corrupt or hostile encoding could produce — is unset, and -// takes part in no containment. +// Range is the contiguous run of term's blocks lo..hi. It is not validated: an inverted or +// zero-touching range is simply not [Interval.Valid]. +func Range(term, lo, hi uint64) Interval { + return Interval{Min: Block{Term: term, N: lo}, Max: Block{Term: term, N: hi}} +} + +// Valid reports whether the interval names a real set of blocks: every held run lies within one +// term and starts at number 1 or above, the bounds are ordered, and the gap list is canonical. +// Everything else — the zero value of a pre-v5 entry, and any inversion or denormalization a +// corrupt or hostile encoding could produce — is unset, and takes part in no containment. func (iv Interval) Valid() bool { - if iv.Min < 1 || iv.Min > iv.Max { + if iv.Max.less(iv.Min) { return false } - prev := iv.Min + lo := iv.Min for _, g := range iv.Gaps { // Strictly inside the bounds, and separated from the previous gap by at least one held // block: otherwise the set has different bounds, or a shorter gap list describes it. - if g.Min > g.Max || g.Min <= prev || g.Max >= iv.Max { + if g.Max.less(g.Min) || !lo.less(g.Min) || !g.Max.less(iv.Max) { return false } - prev = g.Max + 1 + if !validRun(lo, g.Min.prev()) { + return false + } + + lo = g.Max.next() } - return true + return validRun(lo, iv.Max) } +func validRun(lo, hi Block) bool { return lo.N >= 1 && lo.Term == hi.Term && !hi.less(lo) } + // Contains reports whether iv covers every block o covers. Both intervals must be valid: an unset // interval neither contains nor is contained, which is the fallback to exact-prefix matching for // parts written before v5. @@ -76,15 +157,19 @@ func (iv Interval) Contains(o Interval) bool { return false } - held := iv.runs() - i := 0 + i, n := 0, iv.nruns() - for _, r := range o.runs() { - for i < len(held) && held[i].Max < r.Min { + for j := range o.nruns() { + r := o.run(j) + for i < n && iv.run(i).Max.less(r.Min) { i++ } - if i >= len(held) || held[i].Min > r.Min || held[i].Max < r.Max { + if i >= n { + return false + } + + if h := iv.run(i); r.Min.less(h.Min) || h.Max.less(r.Max) { return false } } @@ -101,7 +186,7 @@ func (iv Interval) Union(o Interval) Interval { case !iv.Valid(): return o default: - return fromRuns(append(iv.runs(), o.runs()...)) + return fromRuns(o.appendRuns(iv.appendRuns(make([]Gap, 0, iv.nruns()+o.nruns())))) } } @@ -118,9 +203,10 @@ func (iv Interval) Len() uint64 { return 0 } - n := iv.Max - iv.Min + 1 - for _, g := range iv.Gaps { - n -= g.Max - g.Min + 1 + var n uint64 + for i := range iv.nruns() { + r := iv.run(i) + n += r.Max.N - r.Min.N + 1 } return n @@ -128,66 +214,82 @@ func (iv Interval) Len() uint64 { // Each calls fn for every block the interval covers, in ascending order, stopping early if fn // returns false. It is how a caller enumerates the members of a split group it is missing. -func (iv Interval) Each(fn func(uint64) bool) { +func (iv Interval) Each(fn func(Block) bool) { if !iv.Valid() { return } - for _, r := range iv.runs() { - for b := r.Min; b <= r.Max; b++ { - if !fn(b) { + for i := range iv.nruns() { + r := iv.run(i) + for n := r.Min.N; ; n++ { + if !fn(Block{Term: r.Min.Term, N: n}) { return } + + if n == r.Max.N { + break + } } } } -// runs returns the ascending, disjoint runs of blocks the interval holds. It assumes validity. -func (iv Interval) runs() []Gap { - out := make([]Gap, 0, len(iv.Gaps)+1) +// nruns and run enumerate the ascending, disjoint runs of blocks the interval holds, without +// materializing them: the relations over identity run per entry on every commit. They assume +// validity. +func (iv Interval) nruns() int { return len(iv.Gaps) + 1 } - lo := iv.Min - for _, g := range iv.Gaps { - out = append(out, Gap{Min: lo, Max: g.Min - 1}) - lo = g.Max + 1 +func (iv Interval) run(i int) Gap { + r := Gap{Min: iv.Min, Max: iv.Max} + if i > 0 { + r.Min = iv.Gaps[i-1].Max.next() + } + + if i < len(iv.Gaps) { + r.Max = iv.Gaps[i].Min.prev() + } + + return r +} + +func (iv Interval) appendRuns(dst []Gap) []Gap { + for i := range iv.nruns() { + dst = append(dst, iv.run(i)) } - return append(out, Gap{Min: lo, Max: iv.Max}) + return dst } // fromRuns is the inverse of [Interval.runs] over an arbitrary run list: it sorts, coalesces and -// derives the canonical bounds-and-gaps form. Runs naming block 0 make the whole set unset, as -// they do everywhere else. +// derives the canonical bounds-and-gaps form. A run naming a block numbered 0 makes the whole set +// unset, as it does everywhere else. func fromRuns(runs []Gap) Interval { if len(runs) == 0 { return Interval{} } slices.SortFunc(runs, func(a, b Gap) int { - switch { - case a.Min != b.Min: - return int(int64(a.Min) - int64(b.Min)) - case a.Max < b.Max: - return -1 - case a.Max > b.Max: - return 1 - default: - return 0 + if c := a.Min.Compare(b.Min); c != 0 { + return c } - }) - if runs[0].Min == 0 { - return Interval{} - } + return a.Max.Compare(b.Max) + }) merged := runs[:1] + for _, r := range runs { + if r.Min.N == 0 { + return Interval{} + } + } + for _, r := range runs[1:] { last := &merged[len(merged)-1] - // r.Min-1, not last.Max+1: the top of the number space is a legal block, and last.Max+1 - // would wrap to 0 there and split a run that is really contiguous. - if r.Min-1 <= last.Max { - last.Max = max(last.Max, r.Max) + // r.Min.prev(), not last.Max.next(): the top of a term's number space is a legal block, + // and last.Max.next() would carry into the next term there and split a run that is + // really contiguous. + if !last.Max.less(r.Min.prev()) { + last.Max = MaxBlock(last.Max, r.Max) continue } @@ -197,7 +299,7 @@ func fromRuns(runs []Gap) Interval { iv := Interval{Min: merged[0].Min, Max: merged[len(merged)-1].Max} for i := 1; i < len(merged); i++ { - iv.Gaps = append(iv.Gaps, Gap{Min: merged[i-1].Max + 1, Max: merged[i].Min - 1}) + iv.Gaps = append(iv.Gaps, Gap{Min: merged[i-1].Max.next(), Max: merged[i].Min.prev()}) } return iv @@ -226,10 +328,11 @@ func (c Claim) Valid() bool { return c.Blocks.Valid() && c.Group.Valid() } func (c Claim) Equal(o Claim) bool { return c.Blocks.Equal(o.Blocks) && c.Group.Equal(o.Group) } // Supersedes reports whether e's data wholly subsumes o's: e sits at a higher merge level and -// covers every block o covers, or, for a split-group member, the whole ancestry its group claims. -// It is decidable from identity alone — no index comparison, no bookkeeping — which is what lets a -// repair terminate: by the time a want is serviced the data may exist only inside a merged -// successor, and that successor has to count as satisfaction. +// covers every block o covers, or, for a split-group member, the whole ancestry its group claims; +// or e is o's identity written by a later tenure. It is decidable from identity alone — no index +// comparison, no bookkeeping — which is what lets a repair terminate: by the time a want is +// serviced the data may exist only inside a merged successor, and that successor has to count as +// satisfaction. // // A split merge's fragment covers only its own fresh blocks and supersedes nothing, however much of // an input it happens to hold; the group's joint claim is resolved against a whole index by @@ -248,25 +351,32 @@ func (e Entry) Supersedes(o Entry) bool { return own.Subsumes(e, o) } -// NextBlock returns the block number the next part committed to this index takes: one above the -// highest block ever allocated under this prefix. +// NextBlock returns the block the next part committed to this index under term takes: one above +// the highest block ever allocated under this prefix, within term. // // Allocation is the shard owner's alone, out of its own index, and is claimed by the same CAS // commit that adds the part — so it needs no coordination and works with the cluster layer -// absent. Two writers racing a handoff resolve through that CAS: one commit lands, and the loser -// re-reads and re-allocates above the winner. +// absent. Two writers racing over one index resolve through that CAS: one commit lands, and the +// loser re-reads and re-allocates above the winner. Two tenures that never see each other's +// index — a displaced owner, replicas whose lineages diverged — cannot collide at all, because each +// allocates only under its own term. +// +// A term below the highest one this index has allocated under continues that term's sequence +// instead. Only a writer with no cluster — term 0, the only writer of its prefix — reaches that, +// and continuing is what keeps it from renumbering over blocks a term-0 index retired long ago. A +// clustered writer below the index's term is refused before it allocates (see [CheckTenure]). // // The high-water mark is carried in the index rather than recomputed from the live set, because // the live set shrinks: retention that empties a shard, or a compaction that retires every part // covering a range, would otherwise rewind the counter and hand a new part an identity an expired // one held. Outstanding wants and unrealized group claims still count towards it, so an index // written before the mark existed does not renumber over a part it is still owed. -func (ix *Index) NextBlock() uint64 { - maxBlock := ix.AllocatedBlocks +func (ix *Index) NextBlock(term uint64) Block { + top := ix.AllocatedBlocks bump := func(iv Interval) { - if iv.Valid() && iv.Max > maxBlock { - maxBlock = iv.Max + if iv.Valid() { + top = MaxBlock(top, iv.Max) } } @@ -280,7 +390,11 @@ func (ix *Index) NextBlock() uint64 { bump(ix.Wanted[i].Claim.Group) } - return maxBlock + 1 + if top.Term < term { + return Block{Term: term, N: 1} + } + + return Block{Term: top.Term, N: top.N + 1} } // Covered is the set of blocks the index's data-bearing parts hold between them, including the @@ -290,23 +404,27 @@ func (ix *Index) Covered() Interval { return ix.covered(Entry.Data) } func (ix *Index) covered(admit func(Entry) bool) Interval { var ( - held Interval + runs = make([]Gap, 0, len(ix.Entries)) claims []Claim ) for i := range ix.Entries { - e := ix.Entries[i] - if !admit(e) { + e := &ix.Entries[i] + if !admit(*e) { continue } - held = held.Union(e.Blocks) + if e.Blocks.Valid() { + runs = e.Blocks.appendRuns(runs) + } if e.Claim.Valid() { claims = append(claims, e.Claim) } } + held := fromRuns(runs) + // A group whose members were themselves split resolves only once the inner group has, so the // pass repeats while it keeps realizing claims. Each round retires at least one claim, so it // terminates in at most len(claims) rounds. @@ -361,6 +479,8 @@ func (ix *Index) satisfying(w Want, admit func(Entry) bool) (Entry, bool) { found bool ) + owed := w.Entry() + for i := range ix.Entries { e := ix.Entries[i] if !admit(e) { @@ -371,7 +491,7 @@ func (ix *Index) satisfying(w Want, admit func(Entry) bool) (Entry, bool) { return e, true } - if !e.Blocks.Contains(w.Blocks) && !e.Supersedes(w.Entry()) { + if !e.Blocks.Contains(w.Blocks) && !e.Supersedes(owed) { continue } @@ -419,7 +539,7 @@ func (ix *Index) jointlySatisfying(w Want, admit func(Entry) bool) (Entry, bool) // It is what makes a group repairable one member at a time. A want naming a pre-split part is // answered by no single peer entry, so repair asks for the group's blocks instead, and the peer // resolves each of those against its own index by ordinary containment. -func (ix *Index) Missing(w Want) []uint64 { +func (ix *Index) Missing(w Want) []Block { if !w.Blocks.Valid() { return nil } @@ -429,7 +549,7 @@ func (ix *Index) Missing(w Want) []uint64 { return nil } - var out []uint64 + var out []Block for i := range ix.Entries { c := ix.Entries[i].Claim @@ -437,8 +557,8 @@ func (ix *Index) Missing(w Want) []uint64 { continue } - c.Group.Each(func(b uint64) bool { - if !held.Contains(Blocks(b)) && !slices.Contains(out, b) { + c.Group.Each(func(b Block) bool { + if !held.Contains(Interval{Min: b, Max: b}) && !slices.Contains(out, b) { out = append(out, b) } @@ -446,7 +566,7 @@ func (ix *Index) Missing(w Want) []uint64 { }) } - slices.Sort(out) + slices.SortFunc(out, Block.Compare) return out } @@ -547,3 +667,6 @@ func (l Lineage) Subsumed(live, added []Entry) map[string]struct{} { return out } + +// Single is the interval holding b alone. +func Single(b Block) Interval { return Interval{Min: b, Max: b} } diff --git a/backend/bucketindex/block_test.go b/backend/bucketindex/block_test.go index b3a51101..78d64d8f 100644 --- a/backend/bucketindex/block_test.go +++ b/backend/bucketindex/block_test.go @@ -18,11 +18,15 @@ func TestIntervalValid(t *testing.T) { want bool }{ {"zero", bucketindex.Interval{}, false}, - {"block zero", bucketindex.Interval{Min: 0, Max: 5}, false}, - {"max zero", bucketindex.Interval{Min: 5, Max: 0}, false}, - {"inverted", bucketindex.Interval{Min: 9, Max: 3}, false}, - {"single", bucketindex.Interval{Min: 1, Max: 1}, true}, - {"range", bucketindex.Interval{Min: 3, Max: 9}, true}, + {"block zero", bucketindex.Range(0, 0, 5), false}, + {"max zero", bucketindex.Range(0, 5, 0), false}, + {"inverted", bucketindex.Range(0, 9, 3), false}, + {"single", bucketindex.Range(0, 1, 1), true}, + {"range", bucketindex.Range(0, 3, 9), true}, + {"term range", bucketindex.Range(7, 3, 9), true}, + {"run across terms", bucketindex.Interval{Min: bucketindex.Block{Term: 1, N: 5}, Max: bucketindex.Block{Term: 2, N: 3}}, false}, + {"run from number zero", bucketindex.Range(4, 0, 3), false}, + {"two terms", bucketindex.TermBlocks(1, 5).Union(bucketindex.TermBlocks(2, 1)), true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -45,7 +49,7 @@ func TestIntervalEqual(t *testing.T) { {"unset", bucketindex.Interval{}, bucketindex.Interval{}, true}, {"single", bucketindex.Blocks(3), bucketindex.Blocks(3), true}, {"gapped", gapped, bucketindex.Blocks(6, 5, 2, 1), true}, - {"nil and empty gaps", bucketindex.Interval{Min: 1, Max: 2}, bucketindex.Interval{Min: 1, Max: 2, Gaps: []bucketindex.Gap{}}, true}, + {"nil and empty gaps", bucketindex.Range(0, 1, 2), bucketindex.Interval{Min: bucketindex.Block{N: 1}, Max: bucketindex.Block{N: 2}, Gaps: []bucketindex.Gap{}}, true}, {"min", bucketindex.Blocks(3), bucketindex.Blocks(4), false}, {"max", bucketindex.Blocks(3, 4), bucketindex.Blocks(3, 4, 5), false}, {"gap", gapped, bucketindex.Blocks(1, 2, 3, 5, 6), false}, @@ -78,7 +82,7 @@ func TestIntervalZeroContainsNothing(t *testing.T) { t.Parallel() var unset bucketindex.Interval - set := bucketindex.Interval{Min: 1, Max: 10} + set := bucketindex.Range(0, 1, 10) assert.False(t, unset.Contains(set)) assert.False(t, set.Contains(unset)) @@ -95,13 +99,13 @@ func TestIntervalContains(t *testing.T) { inner bucketindex.Interval contains bool }{ - {"identical", bucketindex.Interval{Min: 3, Max: 3}, bucketindex.Interval{Min: 3, Max: 3}, true}, - {"strict", bucketindex.Interval{Min: 1, Max: 10}, bucketindex.Interval{Min: 4, Max: 6}, true}, - {"left edge", bucketindex.Interval{Min: 1, Max: 10}, bucketindex.Interval{Min: 1, Max: 2}, true}, - {"right edge", bucketindex.Interval{Min: 1, Max: 10}, bucketindex.Interval{Min: 9, Max: 10}, true}, - {"overhang", bucketindex.Interval{Min: 2, Max: 10}, bucketindex.Interval{Min: 1, Max: 10}, false}, - {"disjoint", bucketindex.Interval{Min: 1, Max: 2}, bucketindex.Interval{Min: 3, Max: 4}, false}, - {"inverted outer", bucketindex.Interval{Min: 10, Max: 1}, bucketindex.Interval{Min: 3, Max: 4}, false}, + {"identical", bucketindex.Range(0, 3, 3), bucketindex.Range(0, 3, 3), true}, + {"strict", bucketindex.Range(0, 1, 10), bucketindex.Range(0, 4, 6), true}, + {"left edge", bucketindex.Range(0, 1, 10), bucketindex.Range(0, 1, 2), true}, + {"right edge", bucketindex.Range(0, 1, 10), bucketindex.Range(0, 9, 10), true}, + {"overhang", bucketindex.Range(0, 2, 10), bucketindex.Range(0, 1, 10), false}, + {"disjoint", bucketindex.Range(0, 1, 2), bucketindex.Range(0, 3, 4), false}, + {"inverted outer", bucketindex.Range(0, 10, 1), bucketindex.Range(0, 3, 4), false}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -118,17 +122,17 @@ func TestIntervalUnion(t *testing.T) { name string a, b, ab bucketindex.Interval }{ - {"adjacent", bucketindex.Interval{Min: 1, Max: 1}, bucketindex.Interval{Min: 2, Max: 2}, bucketindex.Interval{Min: 1, Max: 2}}, + {"adjacent", bucketindex.Range(0, 1, 1), bucketindex.Range(0, 2, 2), bucketindex.Range(0, 1, 2)}, // The union is a set, not a hull: blocks 2 to 4 belong to neither side and the result must // not claim them, or a merge of a lost part's neighbors would discharge the want for it. { "gapped", - bucketindex.Interval{Min: 1, Max: 1}, bucketindex.Interval{Min: 5, Max: 7}, - bucketindex.Interval{Min: 1, Max: 7, Gaps: []bucketindex.Gap{{Min: 2, Max: 4}}}, + bucketindex.Range(0, 1, 1), bucketindex.Range(0, 5, 7), + bucketindex.Interval{Min: bucketindex.Block{N: 1}, Max: bucketindex.Block{N: 7}, Gaps: []bucketindex.Gap{{Min: bucketindex.Block{N: 2}, Max: bucketindex.Block{N: 4}}}}, }, - {"nested", bucketindex.Interval{Min: 1, Max: 9}, bucketindex.Interval{Min: 4, Max: 5}, bucketindex.Interval{Min: 1, Max: 9}}, - {"unset right", bucketindex.Interval{Min: 2, Max: 3}, bucketindex.Interval{}, bucketindex.Interval{Min: 2, Max: 3}}, - {"unset left", bucketindex.Interval{}, bucketindex.Interval{Min: 2, Max: 3}, bucketindex.Interval{Min: 2, Max: 3}}, + {"nested", bucketindex.Range(0, 1, 9), bucketindex.Range(0, 4, 5), bucketindex.Range(0, 1, 9)}, + {"unset right", bucketindex.Range(0, 2, 3), bucketindex.Interval{}, bucketindex.Range(0, 2, 3)}, + {"unset left", bucketindex.Interval{}, bucketindex.Range(0, 2, 3), bucketindex.Range(0, 2, 3)}, {"both unset", bucketindex.Interval{}, bucketindex.Interval{}, bucketindex.Interval{}}, } for _, tc := range cases { @@ -143,17 +147,48 @@ func TestIntervalUnion(t *testing.T) { func TestIntervalBlocks(t *testing.T) { t.Parallel() - assert.EqualValues(t, 1, bucketindex.Interval{Min: 7, Max: 7}.Len()) - assert.EqualValues(t, 8, bucketindex.Interval{Min: 3, Max: 10}.Len()) + assert.EqualValues(t, 1, bucketindex.Range(0, 7, 7).Len()) + assert.EqualValues(t, 8, bucketindex.Range(0, 3, 10).Len()) +} + +// TestIntervalAcrossTerms pins the set a merge across tenures holds: each term's blocks, and a gap +// spanning the rest of the lower term's number space and the start of the higher one's. +func TestIntervalAcrossTerms(t *testing.T) { + t.Parallel() + + iv := bucketindex.TermBlocks(1, 5, 6).Union(bucketindex.TermBlocks(2, 1)) + require.True(t, iv.Valid()) + + assert.Equal(t, bucketindex.Interval{ + Min: bucketindex.Block{Term: 1, N: 5}, + Max: bucketindex.Block{Term: 2, N: 1}, + Gaps: []bucketindex.Gap{{ + Min: bucketindex.Block{Term: 1, N: 7}, + Max: bucketindex.Block{Term: 2, N: 0}, + }}, + }, iv) + assert.EqualValues(t, 3, iv.Len()) + assert.True(t, iv.Contains(bucketindex.TermBlocks(2, 1))) + assert.False(t, iv.Contains(bucketindex.TermBlocks(1, 7)), "the rest of term 1 is not held") + assert.False(t, iv.Contains(bucketindex.Blocks(5)), "nor is term 0's block of the same number") + + var seen []bucketindex.Block + + iv.Each(func(b bucketindex.Block) bool { + seen = append(seen, b) + + return len(seen) < 2 + }) + assert.Equal(t, []bucketindex.Block{{Term: 1, N: 5}, {Term: 1, N: 6}}, seen, "Each stops when told to") } func TestSupersedes(t *testing.T) { t.Parallel() flush := func(n uint64) bucketindex.Entry { - return bucketindex.Entry{Prefix: "p", Blocks: bucketindex.Interval{Min: n, Max: n}} + return bucketindex.Entry{Prefix: "p", Blocks: bucketindex.Range(0, n, n)} } - merged := bucketindex.Entry{Prefix: "m", Blocks: bucketindex.Interval{Min: 1, Max: 3}, Level: 1} + merged := bucketindex.Entry{Prefix: "m", Blocks: bucketindex.Range(0, 1, 3), Level: 1} assert.True(t, merged.Supersedes(flush(1))) assert.True(t, merged.Supersedes(flush(3))) @@ -161,7 +196,7 @@ func TestSupersedes(t *testing.T) { assert.False(t, flush(1).Supersedes(merged), "narrower and lower") assert.False(t, merged.Supersedes(merged), "same level does not supersede itself") - sameLevel := bucketindex.Entry{Prefix: "s", Blocks: bucketindex.Interval{Min: 1, Max: 5}} + sameLevel := bucketindex.Entry{Prefix: "s", Blocks: bucketindex.Range(0, 1, 5)} assert.False(t, sameLevel.Supersedes(flush(2)), "containment alone is not supersession") } @@ -171,7 +206,7 @@ func TestSupersedesPreV5(t *testing.T) { t.Parallel() old := bucketindex.Entry{Prefix: "old"} - merged := bucketindex.Entry{Prefix: "m", Blocks: bucketindex.Interval{Min: 1, Max: 100}, Level: 3} + merged := bucketindex.Entry{Prefix: "m", Blocks: bucketindex.Range(0, 1, 100), Level: 3} assert.False(t, merged.Supersedes(old)) assert.False(t, old.Supersedes(merged)) @@ -182,32 +217,32 @@ func TestNextBlock(t *testing.T) { t.Parallel() var ix bucketindex.Index - assert.EqualValues(t, 1, ix.NextBlock(), "block numbering starts at 1") + assert.Equal(t, bucketindex.Block{N: 1}, ix.NextBlock(0), "block numbering starts at 1") ix.Add(bucketindex.Entry{Prefix: "a"}) - assert.EqualValues(t, 1, ix.NextBlock(), "a pre-v5 entry claims no block") + assert.Equal(t, bucketindex.Block{N: 1}, ix.NextBlock(0), "a pre-v5 entry claims no block") - ix.Add(bucketindex.Entry{Prefix: "b", Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 1}) - ix.Add(bucketindex.Entry{Prefix: "c", Blocks: bucketindex.Interval{Min: 5, Max: 5}}) - assert.EqualValues(t, 6, ix.NextBlock()) + ix.Add(bucketindex.Entry{Prefix: "b", Blocks: bucketindex.Range(0, 1, 4), Level: 1}) + ix.Add(bucketindex.Entry{Prefix: "c", Blocks: bucketindex.Range(0, 5, 5)}) + assert.Equal(t, bucketindex.Block{N: 6}, ix.NextBlock(0)) // A want's blocks stay claimed: the part may yet be repaired back in. - ix.RecordWant(bucketindex.Want{Prefix: "d", Blocks: bucketindex.Interval{Min: 9, Max: 9}}) - assert.EqualValues(t, 10, ix.NextBlock()) + ix.RecordWant(bucketindex.Want{Prefix: "d", Blocks: bucketindex.Range(0, 9, 9)}) + assert.Equal(t, bucketindex.Block{N: 10}, ix.NextBlock(0)) } func TestSatisfying(t *testing.T) { t.Parallel() ix := &bucketindex.Index{} - ix.Add(bucketindex.Entry{Prefix: "small", Blocks: bucketindex.Interval{Min: 1, Max: 2}, Level: 1}) - ix.Add(bucketindex.Entry{Prefix: "big", Blocks: bucketindex.Interval{Min: 1, Max: 8}, Level: 2}) + ix.Add(bucketindex.Entry{Prefix: "small", Blocks: bucketindex.Range(0, 1, 2), Level: 1}) + ix.Add(bucketindex.Entry{Prefix: "big", Blocks: bucketindex.Range(0, 1, 8), Level: 2}) - got, ok := ix.Satisfying(bucketindex.Want{Prefix: "gone", Blocks: bucketindex.Interval{Min: 2, Max: 2}}) + got, ok := ix.Satisfying(bucketindex.Want{Prefix: "gone", Blocks: bucketindex.Range(0, 2, 2)}) require.True(t, ok) assert.Equal(t, "big", got.Prefix, "the largest containing part wins") - _, ok = ix.Satisfying(bucketindex.Want{Prefix: "gone", Blocks: bucketindex.Interval{Min: 9, Max: 9}}) + _, ok = ix.Satisfying(bucketindex.Want{Prefix: "gone", Blocks: bucketindex.Range(0, 9, 9)}) assert.False(t, ok, "no part covers the block") // An exact prefix hit satisfies even when the entry carries no interval. @@ -223,19 +258,19 @@ func TestSatisfying(t *testing.T) { func TestSatisfyingTieBreak(t *testing.T) { t.Parallel() - w := bucketindex.Want{Prefix: "gone", Blocks: bucketindex.Interval{Min: 2, Max: 2}} + w := bucketindex.Want{Prefix: "gone", Blocks: bucketindex.Range(0, 2, 2)} byLevel := &bucketindex.Index{} - byLevel.Add(bucketindex.Entry{Prefix: "z", Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 1}) - byLevel.Add(bucketindex.Entry{Prefix: "a", Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 2}) + byLevel.Add(bucketindex.Entry{Prefix: "z", Blocks: bucketindex.Range(0, 1, 4), Level: 1}) + byLevel.Add(bucketindex.Entry{Prefix: "a", Blocks: bucketindex.Range(0, 1, 4), Level: 2}) got, ok := byLevel.Satisfying(w) require.True(t, ok) assert.Equal(t, "a", got.Prefix, "equal width, higher level") byPrefix := &bucketindex.Index{} - byPrefix.Add(bucketindex.Entry{Prefix: "z", Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 1}) - byPrefix.Add(bucketindex.Entry{Prefix: "a", Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 1}) + byPrefix.Add(bucketindex.Entry{Prefix: "z", Blocks: bucketindex.Range(0, 1, 4), Level: 1}) + byPrefix.Add(bucketindex.Entry{Prefix: "a", Blocks: bucketindex.Range(0, 1, 4), Level: 1}) got, ok = byPrefix.Satisfying(w) require.True(t, ok) diff --git a/backend/bucketindex/blockset_test.go b/backend/bucketindex/blockset_test.go index d744cd82..87005fd9 100644 --- a/backend/bucketindex/blockset_test.go +++ b/backend/bucketindex/blockset_test.go @@ -13,10 +13,10 @@ import ( // members is the block set as a plain sorted list — the naive reference the algebra is checked // against, so a bug in the runs-and-gaps encoding cannot hide behind the same bug in the test. -func members(iv bucketindex.Interval) []uint64 { - var out []uint64 +func members(iv bucketindex.Interval) []bucketindex.Block { + var out []bucketindex.Block - iv.Each(func(b uint64) bool { + iv.Each(func(b bucketindex.Block) bool { out = append(out, b) return true @@ -25,16 +25,24 @@ func members(iv bucketindex.Interval) []uint64 { return out } -func randomSet(rnd *rand.Rand, span uint64) ([]uint64, bucketindex.Interval) { - var nums []uint64 - - for b := uint64(1); b <= span; b++ { - if rnd.IntN(3) == 0 { - nums = append(nums, b) +// randomSet draws a set over terms 0 to 2, so runs and gaps also cross the boundary between two +// tenures' number spaces. +func randomSet(rnd *rand.Rand, span uint64) ([]bucketindex.Block, bucketindex.Interval) { + var ( + blocks []bucketindex.Block + iv bucketindex.Interval + ) + + for term := range uint64(3) { + for n := uint64(1); n <= span; n++ { + if rnd.IntN(6) == 0 { + blocks = append(blocks, bucketindex.Block{Term: term, N: n}) + iv = iv.Union(bucketindex.TermBlocks(term, n)) + } } } - return nums, bucketindex.Blocks(nums...) + return blocks, iv } func TestBlocksMatchesTheNaiveSet(t *testing.T) { @@ -82,7 +90,7 @@ func TestUnionMatchesSetUnion(t *testing.T) { an, a := randomSet(rnd, 10) bn, b := randomSet(rnd, 10) - want := slices.Compact(slices.Sorted(slices.Values(slices.Concat(an, bn)))) + want := slices.Compact(slices.SortedFunc(slices.Values(slices.Concat(an, bn)), bucketindex.Block.Compare)) ab := a.Union(b) assert.Equal(t, want, members(ab)) @@ -149,7 +157,7 @@ func TestNoHullClaimsAnUnheldBlock(t *testing.T) { rnd := rand.New(rand.NewPCG(43, 47)) for range 1000 { inputs := make([]bucketindex.Interval, 0, 4) - held := map[uint64]struct{}{} + held := map[bucketindex.Block]struct{}{} var union bucketindex.Interval @@ -165,7 +173,7 @@ func TestNoHullClaimsAnUnheldBlock(t *testing.T) { for _, b := range members(union) { _, ok := held[b] - assert.Truef(t, ok, "block %d is claimed by the union of %v but held by no input", b, inputs) + assert.Truef(t, ok, "block %v is claimed by the union of %v but held by no input", b, inputs) } } } @@ -188,7 +196,7 @@ func TestJointCoverageNeedsEveryMember(t *testing.T) { partial := &bucketindex.Index{Entries: []bucketindex.Entry{frag("a", 4), frag("b", 5)}} _, ok := partial.Satisfying(want) assert.False(t, ok, "two thirds of a group answers nothing") - assert.Equal(t, []uint64{6}, partial.Missing(want), "and the third is what repair must fetch") + assert.Equal(t, []bucketindex.Block{{N: 6}}, partial.Missing(want), "and the third is what repair must fetch") whole := &bucketindex.Index{Entries: []bucketindex.Entry{frag("a", 4), frag("b", 5), frag("c", 6)}} got, ok := whole.Satisfying(want) diff --git a/backend/bucketindex/bucketindex.go b/backend/bucketindex/bucketindex.go index 9942b98d..7dee7b56 100644 --- a/backend/bucketindex/bucketindex.go +++ b/backend/bucketindex/bucketindex.go @@ -42,6 +42,11 @@ type Entry struct { // for every part a merge wrote whole, which is nearly all of them. See [Claim]. Added in // format v6. Claim Claim + // Term is the ownership term of the tenure that wrote this part. It is what tells apart two + // parts of one identity — two owners that merged the same inputs either side of a handoff — and + // resolves them to the later tenure (see [Lineage.Subsumes]). Zero for a writer with no cluster, + // and for every part an index written before format v7 names. Added in format v7. + Term uint64 // Hole marks this entry as an acknowledged loss rather than a part: the writer owed a repair // for these blocks, no owner could supply them, and it committed this in their place so the // obligation stops blocking reads. It names no objects and holds no rows. @@ -85,11 +90,11 @@ type Index struct { // value it read forward and takes the maximum when it rebases on a rival's commit, so it is a // cluster-visible fact rather than a per-node level a restart resets. Added in format v5. LostParts uint64 - // AllocatedBlocks is the highest block number ever handed out under this prefix — the - // allocation high-water mark [Index.NextBlock] runs above. It is a separate number from - // anything the live set says, because the live set shrinks and identity must not: see - // [Index.NextBlock]. Added in format v6. - AllocatedBlocks uint64 + // AllocatedBlocks is the highest block ever handed out under this prefix — the allocation + // high-water mark [Index.NextBlock] runs above. It is separate from anything the live set says, + // because the live set shrinks and identity must not: see [Index.NextBlock]. Added in format v6, + // as a term-0 number; a [Block] since format v7. + AllocatedBlocks Block } // Add inserts e, replacing any existing entry with the same prefix, keeping the index sorted. @@ -136,6 +141,9 @@ func (ix *Index) Overlapping(start, end int64) []Entry { const ( magic0, magic1 = 'B', 'I' + // v7 scopes every block to the ownership term that allocated it (a [Block] is a term and a + // number), and adds the writing tenure's term to entries and wants. A v6 block is read as term 0. + // // v6 turns the block interval into an exact set (bounds plus gaps), adds the split-group claim // to entries and wants, and appends the allocation high-water mark. v5 (the block interval, // level and the wanted list), v4 (the per-writer flush watermarks), v3 (Generation + Removed), @@ -150,7 +158,7 @@ const ( // Reading is backward compatible; writing is not. [Decode] rejects any version above this one, // so a node on pre-v5 code cannot read an index this one writes: every node that reads a given // index must be upgraded together. See backend/ARCH.md for the blast radius per deployment. - version = 6 + version = 7 ) // AppendBinary appends the versioned binary encoding of the index to dst (append-style for @@ -168,6 +176,7 @@ func (ix *Index) AppendBinary(dst []byte) []byte { dst = binary.AppendUvarint(dst, uint64(e.Level)) dst = binary.AppendUvarint(dst, entryFlags(*e)) dst = appendClaim(dst, e.Claim) + dst = binary.AppendUvarint(dst, e.Term) } dst = binary.AppendUvarint(dst, ix.FlushedEpoch) @@ -205,10 +214,12 @@ func (ix *Index) AppendBinary(dst []byte) []byte { dst = binary.AppendUvarint(dst, w.Generation.Term) dst = binary.AppendUvarint(dst, w.Generation.Counter) dst = appendClaim(dst, w.Claim) + dst = binary.AppendUvarint(dst, w.Term) } dst = binary.AppendUvarint(dst, ix.LostParts) - dst = binary.AppendUvarint(dst, ix.AllocatedBlocks) + dst = binary.AppendUvarint(dst, ix.AllocatedBlocks.Term) + dst = binary.AppendUvarint(dst, ix.AllocatedBlocks.N) return dst } @@ -295,7 +306,7 @@ func Decode(data []byte) (*Index, error) { } // decodeTail parses the v5+ tail: the outstanding wants, the loss counter, and the v6+ allocation -// high-water mark. +// high-water mark (a term-0 number in v6, a [Block] since v7). func decodeTail(ix *Index, buf []byte, ver uint8) error { wanted, rest, err := decodeWants(buf, ver) if err != nil { @@ -317,7 +328,13 @@ func decodeTail(ix *Index, buf []byte, ver uint8) error { return nil } - if ix.AllocatedBlocks, _, ok = readUvarint(rest); !ok { + if ver >= 7 { + if ix.AllocatedBlocks.Term, rest, ok = readUvarint(rest); !ok { + return errors.Wrap(ErrCorrupt, "bad allocated term") + } + } + + if ix.AllocatedBlocks.N, _, ok = readUvarint(rest); !ok { return errors.Wrap(ErrCorrupt, "bad allocated blocks") } @@ -412,7 +429,13 @@ func decodeBlockIdentity(buf []byte, ver uint8, e *Entry) ([]byte, bool) { e.Hole = flags&entryFlagHole != 0 if ver >= 6 { - if e.Claim, buf, ok = readClaim(buf); !ok { + if e.Claim, buf, ok = readClaim(buf, ver); !ok { + return nil, false + } + } + + if ver >= 7 { + if e.Term, buf, ok = readUvarint(buf); !ok { return nil, false } } @@ -420,27 +443,54 @@ func decodeBlockIdentity(buf []byte, ver uint8, e *Entry) ([]byte, bool) { return buf, true } -// appendInterval writes a block set. An unset one costs a single zero byte, and the gap list is -// written only for a set that has one, so the ordinary contiguous part pays three bytes. +// appendInterval writes a block set. An unset one costs a single zero byte. A set leads with its +// lowest block's number, which a valid set never has at 0, so the zero byte stays unambiguous. The +// upper bound's term is written as a delta, and the gap list only for a set that has one, so the +// ordinary contiguous part pays for its term once. func appendInterval(dst []byte, iv Interval) []byte { if !iv.Valid() { return binary.AppendUvarint(dst, 0) } - dst = binary.AppendUvarint(dst, iv.Min) - dst = binary.AppendUvarint(dst, iv.Max) + dst = binary.AppendUvarint(dst, iv.Min.N) + dst = binary.AppendUvarint(dst, iv.Min.Term) + dst = binary.AppendUvarint(dst, iv.Max.Term-iv.Min.Term) + dst = binary.AppendUvarint(dst, iv.Max.N) dst = binary.AppendUvarint(dst, uint64(len(iv.Gaps))) for _, g := range iv.Gaps { - dst = binary.AppendUvarint(dst, g.Min) - dst = binary.AppendUvarint(dst, g.Max) + dst = appendBlock(dst, g.Min) + dst = appendBlock(dst, g.Max) } return dst } -// readInterval parses a block set. A v5 encoding carries bounds only and is read as the contiguous -// set they describe — the meaning a hull had when it was written. +func appendBlock(dst []byte, b Block) []byte { + dst = binary.AppendUvarint(dst, b.Term) + + return binary.AppendUvarint(dst, b.N) +} + +func readBlock(buf []byte) (Block, []byte, bool) { + var ( + b Block + ok bool + ) + + if b.Term, buf, ok = readUvarint(buf); !ok { + return Block{}, nil, false + } + + if b.N, buf, ok = readUvarint(buf); !ok { + return Block{}, nil, false + } + + return b, buf, true +} + +// readInterval parses a block set. Blocks before v7 are term 0. A v5 encoding carries bounds only +// and is read as the contiguous set they describe — the meaning a hull had when it was written. // // A set that does not round-trip to its canonical form is rejected rather than normalized: two // encodings of one set would break both equality and the encode∘decode identity, and a @@ -451,23 +501,43 @@ func readInterval(buf []byte, ver uint8) (Interval, []byte, bool) { ok bool ) - if iv.Min, buf, ok = readUvarint(buf); !ok { + if iv.Min.N, buf, ok = readUvarint(buf); !ok { return Interval{}, nil, false } if ver < 6 { - if iv.Max, buf, ok = readUvarint(buf); !ok { + if iv.Max.N, buf, ok = readUvarint(buf); !ok { return Interval{}, nil, false } + // A hull that names no real set — inverted, or touching block 0 — is unset, as it would be + // written back; keeping it verbatim would make decode∘encode differ from the identity. + if !iv.Valid() { + return Interval{}, buf, true + } + return iv, buf, true } - if iv.Min == 0 { + if iv.Min.N == 0 { return Interval{}, buf, true } - if iv.Max, buf, ok = readUvarint(buf); !ok { + if ver >= 7 { + var span uint64 + + if iv.Min.Term, buf, ok = readUvarint(buf); !ok { + return Interval{}, nil, false + } + + if span, buf, ok = readUvarint(buf); !ok || span > math.MaxUint64-iv.Min.Term { + return Interval{}, nil, false + } + + iv.Max.Term = iv.Min.Term + span + } + + if iv.Max.N, buf, ok = readUvarint(buf); !ok { return Interval{}, nil, false } @@ -481,11 +551,7 @@ func readInterval(buf []byte, ver uint8) (Interval, []byte, bool) { for range n { var g Gap - if g.Min, buf, ok = readUvarint(buf); !ok { - return Interval{}, nil, false - } - - if g.Max, buf, ok = readUvarint(buf); !ok { + if buf, ok = readGap(buf, ver, &g); !ok { return Interval{}, nil, false } @@ -500,6 +566,28 @@ func readInterval(buf []byte, ver uint8) (Interval, []byte, bool) { return iv, buf, true } +func readGap(buf []byte, ver uint8, g *Gap) ([]byte, bool) { + var ok bool + + if ver >= 7 { + if g.Min, buf, ok = readBlock(buf); !ok { + return nil, false + } + + g.Max, buf, ok = readBlock(buf) + + return buf, ok + } + + if g.Min.N, buf, ok = readUvarint(buf); !ok { + return nil, false + } + + g.Max.N, buf, ok = readUvarint(buf) + + return buf, ok +} + func appendClaim(dst []byte, c Claim) []byte { if !c.Valid() { return binary.AppendUvarint(dst, 0) @@ -511,7 +599,7 @@ func appendClaim(dst []byte, c Claim) []byte { return appendInterval(dst, c.Group) } -func readClaim(buf []byte) (Claim, []byte, bool) { +func readClaim(buf []byte, ver uint8) (Claim, []byte, bool) { present, buf, ok := readUvarint(buf) if !ok || present > 1 { return Claim{}, nil, false @@ -523,11 +611,11 @@ func readClaim(buf []byte) (Claim, []byte, bool) { var c Claim - if c.Blocks, buf, ok = readInterval(buf, version); !ok { + if c.Blocks, buf, ok = readInterval(buf, ver); !ok { return Claim{}, nil, false } - if c.Group, buf, ok = readInterval(buf, version); !ok { + if c.Group, buf, ok = readInterval(buf, ver); !ok { return Claim{}, nil, false } @@ -595,11 +683,17 @@ func decodeWants(buf []byte, ver uint8) ([]Want, []byte, error) { } if ver >= 6 { - if w.Claim, buf, ok = readClaim(buf); !ok { + if w.Claim, buf, ok = readClaim(buf, ver); !ok { return nil, nil, errors.Wrap(ErrCorrupt, "bad want claim") } } + if ver >= 7 { + if w.Term, buf, ok = readUvarint(buf); !ok { + return nil, nil, errors.Wrap(ErrCorrupt, "bad want tenure") + } + } + out = append(out, w) } diff --git a/backend/bucketindex/bucketindex_test.go b/backend/bucketindex/bucketindex_test.go index ba1c7c25..7e007b08 100644 --- a/backend/bucketindex/bucketindex_test.go +++ b/backend/bucketindex/bucketindex_test.go @@ -80,11 +80,12 @@ func TestGoldenEncoding(t *testing.T) { FlushedEpoch: 3, Generation: bucketindex.Generation{Term: 4, Counter: 5}, } - // magic 'B','I', version 6, count 1, len 1, 'a', zigzag(1)=2, zigzag(2)=4, unset blocks 0, - // level 0, flags 0, no claim 0, flushedEpoch 3, generation term 4, generation counter 5, - // removal count 0, writer-epoch count 0, want count 0, lost parts 0, allocated blocks 0. + // magic 'B','I', version 7, count 1, len 1, 'a', zigzag(1)=2, zigzag(2)=4, unset blocks 0, + // level 0, flags 0, no claim 0, writer term 0, flushedEpoch 3, generation term 4, generation + // counter 5, removal count 0, writer-epoch count 0, want count 0, lost parts 0, allocated + // blocks' term 0 and number 0. assert.Equal(t, - []byte{'B', 'I', 6, 1, 1, 'a', 2, 4, 0, 0, 0, 0, 3, 4, 5, 0, 0, 0, 0, 0}, + []byte{'B', 'I', 7, 1, 1, 'a', 2, 4, 0, 0, 0, 0, 0, 3, 4, 5, 0, 0, 0, 0, 0, 0}, ix.AppendBinary(nil)) } @@ -210,6 +211,11 @@ func FuzzDecode(f *testing.F) { {Writer: "node-a", Epoch: 3, Generation: bucketindex.Generation{Term: 1, Counter: 2}}, }, }).AppendBinary(nil)) + f.Add(fullIndex().AppendBinary(nil)) + f.Add([]byte{ + 'B', 'I', 6, 1, 1, 'a', 2, 4, 1, 4, 1, 2, 3, 2, 0, 1, 6, 7, 0, 1, 4, 0, 3, 4, 5, 1, 1, 'r', 6, 7, + 1, 1, 'w', 8, 9, 10, 1, 1, 'x', 5, 5, 0, 1, 26, 28, 11, 12, 0, 15, 16, + }) f.Fuzz(func(t *testing.T, data []byte) { ix, err := bucketindex.Decode(data) @@ -223,5 +229,6 @@ func FuzzDecode(f *testing.F) { assert.Equal(t, ix.Entries, again.Entries) assert.Equal(t, ix.Epochs, again.Epochs) assert.Equal(t, ix.Wanted, again.Wanted) + assert.Equal(t, ix.AllocatedBlocks, again.AllocatedBlocks) }) } diff --git a/backend/bucketindex/format_test.go b/backend/bucketindex/format_test.go index 4e02d9bf..979e1f8f 100644 --- a/backend/bucketindex/format_test.go +++ b/backend/bucketindex/format_test.go @@ -16,11 +16,11 @@ func fullIndex() *bucketindex.Index { return &bucketindex.Index{ Entries: []bucketindex.Entry{ { - Prefix: "a", MinTime: 1, MaxTime: 2, Level: 2, - Blocks: bucketindex.Interval{Min: 1, Max: 4, Gaps: []bucketindex.Gap{{Min: 2, Max: 3}}}, + Prefix: "a", MinTime: 1, MaxTime: 2, Level: 2, Term: 3, + Blocks: bucketindex.TermBlocks(1, 1, 4).Union(bucketindex.TermBlocks(2, 1)), Claim: bucketindex.Claim{ - Blocks: bucketindex.Interval{Min: 6, Max: 7}, - Group: bucketindex.Interval{Min: 1, Max: 4}, + Blocks: bucketindex.Range(1, 6, 7), + Group: bucketindex.Range(2, 1, 4), }, }, }, @@ -33,40 +33,46 @@ func fullIndex() *bucketindex.Index { Wanted: []bucketindex.Want{ { Prefix: "x", - Blocks: bucketindex.Interval{Min: 5, Max: 5}, + Blocks: bucketindex.Range(2, 5, 5), Level: 1, + Term: 2, MinTime: 13, MaxTime: 14, Generation: bucketindex.Generation{Term: 11, Counter: 12}, }, }, LostParts: 15, - AllocatedBlocks: 16, + AllocatedBlocks: bucketindex.Block{Term: 2, N: 16}, } } -// TestGoldenV6 pins the v6 byte layout: an accidental reordering or a dropped field breaks here +// TestGoldenV7 pins the v7 byte layout: an accidental reordering or a dropped field breaks here // before it breaks a deployment. -func TestGoldenV6(t *testing.T) { +func TestGoldenV7(t *testing.T) { t.Parallel() want := []byte{ - 'B', 'I', 6, - // one entry: prefix, zigzag times, blocks {1} ∪ {4} as bounds 1..4 with one gap 2..3, - // level 2, flags, a claim on blocks [6,7] held by group [1,4]. - 1, 1, 'a', 2, 4, 1, 4, 1, 2, 3, 2, 0, 1, 6, 7, 0, 1, 4, 0, + 'B', 'I', 7, + // one entry: prefix, zigzag times, + 1, 1, 'a', 2, 4, + // blocks {1:1, 1:4, 2:1}: lowest number 1, its term 1, top term +1, top number 1, and two + // gaps, 1:2..1:3 and 1:5..2:0 — the rest of term 1's number space, + 1, 1, 1, 1, 2, 1, 2, 1, 3, 1, 5, 2, 0, + // level 2, flags, a claim on term 1's [6,7] held by term 2's group [1,4], the writer's term, + 2, 0, 1, 6, 1, 0, 7, 0, 1, 2, 0, 4, 0, 3, 3, // flushed epoch 4, 5, // generation 1, 1, 'r', 6, 7, // one removal 1, 1, 'w', 8, 9, 10, // one writer epoch - 1, 1, 'x', 5, 5, 0, 1, 26, 28, 11, 12, 0, // one want: prefix, blocks, level, times, gen, no claim - 15, // lost parts - 16, // allocated blocks + // one want: prefix, blocks {2:5}, level, times, gen, no claim, the writer's term + 1, 1, 'x', 5, 2, 0, 5, 0, 1, 26, 28, 11, 12, 0, 2, + 15, // lost parts + 2, 16, // allocated blocks: term, number } assert.Equal(t, want, fullIndex().AppendBinary(nil)) } -func TestV6RoundTrip(t *testing.T) { +func TestV7RoundTrip(t *testing.T) { t.Parallel() in := fullIndex() @@ -75,6 +81,37 @@ func TestV6RoundTrip(t *testing.T) { assert.Equal(t, in, out) } +// TestDecodeV6Golden pins the migration from v6: every block is term 0, and so are the writers' +// terms and the high-water mark. +func TestDecodeV6Golden(t *testing.T) { + t.Parallel() + + got, err := bucketindex.Decode([]byte{ + 'B', 'I', 6, + // one entry: prefix, zigzag times, blocks {1} ∪ {4} as bounds 1..4 with one gap 2..3, + // level 2, flags, a claim on blocks [6,7] held by group [1,4]. + 1, 1, 'a', 2, 4, 1, 4, 1, 2, 3, 2, 0, 1, 6, 7, 0, 1, 4, 0, + 3, // flushed epoch + 4, 5, // generation + 1, 1, 'r', 6, 7, // one removal + 1, 1, 'w', 8, 9, 10, // one writer epoch + 1, 1, 'x', 5, 5, 0, 1, 26, 28, 11, 12, 0, // one want: prefix, blocks, level, times, gen, no claim + 15, // lost parts + 16, // allocated blocks + }) + require.NoError(t, err) + + want := fullIndex() + want.Entries[0].Term, want.Wanted[0].Term = 0, 0 + want.Entries[0].Blocks = bucketindex.Blocks(1, 4) + want.Entries[0].Claim = bucketindex.Claim{Blocks: bucketindex.Range(0, 6, 7), Group: bucketindex.Range(0, 1, 4)} + want.Wanted[0].Blocks = bucketindex.Blocks(5) + want.AllocatedBlocks = bucketindex.Block{N: 16} + assert.Equal(t, want, got) + assert.Equal(t, bucketindex.Block{N: 17}, got.NextBlock(0), "a writer with no cluster continues term 0") + assert.Equal(t, bucketindex.Block{Term: 9, N: 1}, got.NextBlock(9), "a tenure starts its own sequence") +} + // TestDecodeV5Golden pins the migration: the v5 bytes this reader must keep parsing, and what a v5 // hull becomes. The hull is read as the contiguous set of its bounds — the meaning it had when it // was written — the claim is unset, and the absent high-water mark leaves [Index.NextBlock] running @@ -94,11 +131,15 @@ func TestDecodeV5Golden(t *testing.T) { }) require.NoError(t, err) - assert.Equal(t, bucketindex.Interval{Min: 1, Max: 4}, got.Entries[0].Blocks) + assert.Equal(t, bucketindex.Range(0, 1, 4), got.Entries[0].Blocks) assert.Equal(t, bucketindex.Claim{}, got.Entries[0].Claim) assert.Zero(t, got.AllocatedBlocks) assert.EqualValues(t, 15, got.LostParts) - assert.EqualValues(t, 6, got.NextBlock(), "numbering continues above what a v5 index still names") + assert.Equal(t, bucketindex.Block{N: 6}, got.NextBlock(0), "numbering continues above what a v5 index still names") + + inverted, err := bucketindex.Decode([]byte{'B', 'I', 5, 1, 1, 'a', 2, 4, 9, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0}) + require.NoError(t, err) + assert.Equal(t, bucketindex.Interval{}, inverted.Entries[0].Blocks, "a hull naming nothing reads as unset") } // TestDecodeV4Compat verifies a v4 index — no block identity, no wanted list — still decodes, with @@ -136,6 +177,7 @@ func TestDecodeAllVersions(t *testing.T) { 4: {'B', 'I', 4, 1, 1, 'a', 2, 4, 3, 4, 5, 0, 0}, 5: {'B', 'I', 5, 1, 1, 'a', 2, 4, 0, 0, 0, 0, 3, 4, 5, 0, 0, 0, 0}, 6: {'B', 'I', 6, 1, 1, 'a', 2, 4, 0, 0, 0, 0, 3, 4, 5, 0, 0, 0, 0, 0}, + 7: {'B', 'I', 7, 1, 1, 'a', 2, 4, 0, 0, 0, 0, 0, 3, 4, 5, 0, 0, 0, 0, 0, 0}, } for ver, data := range cases { t.Run(fmt.Sprintf("v%d", ver), func(t *testing.T) { @@ -150,7 +192,7 @@ func TestDecodeAllVersions(t *testing.T) { }) } - _, err := bucketindex.Decode([]byte{'B', 'I', 7, 0}) + _, err := bucketindex.Decode([]byte{'B', 'I', 8, 0}) require.ErrorIs(t, err, bucketindex.ErrCorrupt, "a version this reader does not know is rejected") } @@ -179,6 +221,31 @@ func TestDecodeRejectsCorruptV6(t *testing.T) { } } +// TestDecodeRejectsCorruptV7 covers the fields v7 added: the block terms, the writers' terms and +// the high-water mark's term. +func TestDecodeRejectsCorruptV7(t *testing.T) { + t.Parallel() + + cases := map[string][]byte{ + "missing min term": {'B', 'I', 7, 1, 1, 'a', 2, 4, 1}, + "missing term span": {'B', 'I', 7, 1, 1, 'a', 2, 4, 1, 3}, + "term span overflows": {'B', 'I', 7, 1, 1, 'a', 2, 4, 1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01, 1, 1, 0}, + "run across terms": {'B', 'I', 7, 1, 1, 'a', 2, 4, 5, 1, 1, 3, 0, 0, 0, 0, 0, 3, 4, 5, 0, 0, 0, 0, 0, 0}, + "missing gap block": {'B', 'I', 7, 1, 1, 'a', 2, 4, 1, 1, 0, 4, 1, 1}, + "missing entry term": {'B', 'I', 7, 1, 1, 'a', 2, 4, 0, 0, 0, 0}, + "missing want term": {'B', 'I', 7, 0, 0, 0, 0, 0, 0, 1, 1, 'x', 0, 0, 0, 0, 0, 0, 0}, + "missing mark term": {'B', 'I', 7, 0, 0, 0, 0, 0, 0, 0, 0}, + "missing mark number": {'B', 'I', 7, 0, 0, 0, 0, 0, 0, 0, 0, 5}, + } + for name, data := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + _, err := bucketindex.Decode(data) + require.ErrorIs(t, err, bucketindex.ErrCorrupt) + }) + } +} + func TestDecodeRejectsCorruptV5(t *testing.T) { t.Parallel() @@ -223,7 +290,7 @@ func randomIndex(rnd *rand.Rand) *bucketindex.Index { ix := &bucketindex.Index{ FlushedEpoch: rnd.Uint64(), Generation: bucketindex.Generation{Term: rnd.Uint64(), Counter: rnd.Uint64()}, - AllocatedBlocks: rnd.Uint64(), + AllocatedBlocks: bucketindex.Block{Term: rnd.Uint64(), N: rnd.Uint64()}, } for i := range rnd.IntN(6) { @@ -234,6 +301,7 @@ func randomIndex(rnd *rand.Rand) *bucketindex.Index { Blocks: randomInterval(rnd), Claim: randomClaim(rnd), Level: uint32(rnd.IntN(4)), + Term: rnd.Uint64N(5), }) } @@ -254,6 +322,7 @@ func randomIndex(rnd *rand.Rand) *bucketindex.Index { Prefix: fmt.Sprintf("lost-%02d", i), Blocks: randomInterval(rnd), Claim: randomClaim(rnd), + Term: rnd.Uint64(), Generation: bucketindex.Generation{Term: rnd.Uint64(), Counter: rnd.Uint64()}, }) } @@ -275,59 +344,70 @@ func randomClaim(rnd *rand.Rand) bucketindex.Claim { } // randomInterval covers the boundaries that matter: unset, block 1, a single block, a wide range, -// a gapped set, and the top of the number space. +// a gapped set, one spanning several terms, and the top of the number and term spaces. func randomInterval(rnd *rand.Rand) bucketindex.Interval { - switch rnd.IntN(6) { + switch rnd.IntN(8) { case 5: nums := make([]uint64, 0, 8) for range rnd.IntN(7) + 1 { nums = append(nums, rnd.Uint64N(40)+1) } - return bucketindex.Blocks(nums...) + return bucketindex.TermBlocks(rnd.Uint64N(3), nums...) + case 6: + var iv bucketindex.Interval + for range rnd.IntN(4) + 1 { + iv = iv.Union(bucketindex.TermBlocks(rnd.Uint64(), rnd.Uint64N(40)+1)) + } + + return iv + case 7: + return bucketindex.Range(math.MaxUint64, math.MaxUint64-1, math.MaxUint64) case 0: return bucketindex.Interval{} case 1: - return bucketindex.Interval{Min: 1, Max: 1} + return bucketindex.Range(0, 1, 1) case 2: n := rnd.Uint64N(1000) + 1 - return bucketindex.Interval{Min: n, Max: n} + return bucketindex.Range(rnd.Uint64(), n, n) case 3: lo := rnd.Uint64N(1000) + 1 - return bucketindex.Interval{Min: lo, Max: lo + rnd.Uint64N(1000)} + return bucketindex.Range(0, lo, lo+rnd.Uint64N(1000)) default: - return bucketindex.Interval{Min: math.MaxUint64 - 1, Max: math.MaxUint64} + return bucketindex.Range(0, math.MaxUint64-1, math.MaxUint64) } } // FuzzRoundTrip drives encode∘decode over arbitrary block sets: the entry's own set is built from -// the fuzzer's bytes so gapped, inverted and out-of-range shapes all reach the encoder. +// the fuzzer's bytes so gapped, inverted, out-of-range and multi-term shapes all reach the encoder. +// Each byte is a block: its low 5 bits the number, its high 3 bits added to term. func FuzzRoundTrip(f *testing.F) { - f.Add([]byte{1}, uint32(0), uint64(0), uint64(0), uint64(0)) - f.Add([]byte{}, uint32(0), uint64(1), uint64(1), uint64(0)) - f.Add([]byte{3, 4, 9, 200}, uint32(7), uint64(math.MaxUint64), uint64(math.MaxUint64), uint64(9)) + f.Add([]byte{1}, uint32(0), uint64(0), uint64(0), uint64(0), uint64(0)) + f.Add([]byte{}, uint32(0), uint64(1), uint64(1), uint64(0), uint64(0)) + f.Add([]byte{3, 4, 9, 200}, uint32(7), uint64(math.MaxUint64), uint64(math.MaxUint64), uint64(9), uint64(0)) + f.Add([]byte{0x21, 0x22, 0x41, 0xff}, uint32(1), uint64(5), uint64(5), uint64(3), uint64(math.MaxUint64-8)) - f.Fuzz(func(t *testing.T, blocks []byte, level uint32, wantMin, wantMax, allocated uint64) { - nums := make([]uint64, 0, len(blocks)) + f.Fuzz(func(t *testing.T, blocks []byte, level uint32, wantMin, wantMax, allocated, term uint64) { + var own bucketindex.Interval for _, b := range blocks { - nums = append(nums, uint64(b)) + own = own.Union(bucketindex.TermBlocks(term+uint64(b>>5), uint64(b&0x1f))) } // An interval that names no real set of blocks is written as unset (see // TestInvalidIntervalEncodesAsUnset), so the identity is stated over canonical input. - wanted := bucketindex.Interval{Min: wantMin, Max: wantMax} + wanted := bucketindex.Range(term, wantMin, wantMax) if !wanted.Valid() { wanted = bucketindex.Interval{} } in := &bucketindex.Index{ Entries: []bucketindex.Entry{ - {Prefix: "p", MinTime: -1, MaxTime: 1, Blocks: bucketindex.Blocks(nums...), Level: level}, + {Prefix: "p", MinTime: -1, MaxTime: 1, Blocks: own, Level: level, Term: term}, }, - Wanted: []bucketindex.Want{{Prefix: "w", Blocks: wanted}}, - AllocatedBlocks: allocated, + Wanted: []bucketindex.Want{{Prefix: "w", Blocks: wanted, Term: allocated}}, + AllocatedBlocks: bucketindex.Block{Term: term, N: allocated}, } out, err := bucketindex.Decode(in.AppendBinary(nil)) @@ -344,10 +424,20 @@ func TestInvalidIntervalEncodesAsUnset(t *testing.T) { t.Parallel() cases := map[string]bucketindex.Interval{ - "names block zero": {Min: 0, Max: 1}, - "inverted": {Min: 9, Max: 2}, - "gap outside": {Min: 1, Max: 4, Gaps: []bucketindex.Gap{{Min: 7, Max: 8}}}, - "gaps unordered": {Min: 1, Max: 9, Gaps: []bucketindex.Gap{{Min: 6, Max: 7}, {Min: 3, Max: 4}}}, + "names block zero": bucketindex.Range(0, 0, 1), + "inverted": bucketindex.Range(0, 9, 2), + "run across terms": {Min: bucketindex.Block{Term: 1, N: 1}, Max: bucketindex.Block{Term: 2, N: 1}}, + "gap outside": { + Min: bucketindex.Block{N: 1}, Max: bucketindex.Block{N: 4}, + Gaps: []bucketindex.Gap{{Min: bucketindex.Block{N: 7}, Max: bucketindex.Block{N: 8}}}, + }, + "gaps unordered": { + Min: bucketindex.Block{N: 1}, Max: bucketindex.Block{N: 9}, + Gaps: []bucketindex.Gap{ + {Min: bucketindex.Block{N: 6}, Max: bucketindex.Block{N: 7}}, + {Min: bucketindex.Block{N: 3}, Max: bucketindex.Block{N: 4}}, + }, + }, } for name, iv := range cases { t.Run(name, func(t *testing.T) { diff --git a/backend/bucketindex/generation.go b/backend/bucketindex/generation.go index d63ae7bc..24ff0437 100644 --- a/backend/bucketindex/generation.go +++ b/backend/bucketindex/generation.go @@ -45,9 +45,10 @@ func (g Generation) Zero() bool { return g == Generation{} } // Next returns the generation of this writer's next index write within term. // // A term above the current one restarts the counter — it is a different writer's sequence, and -// the term already orders it above everything the previous one wrote. A term below is a writer -// that has been superseded and does not know it yet; its own counter still advances so that its -// local state stays monotonic, and the term keeps its writes from being adopted anywhere. +// the term already orders it above everything the previous one wrote. A term below keeps the +// higher one, which only a writer with no cluster (term 0) may do: a clustered writer below the +// index's term is refused before it commits ([CheckTenure]), so it never writes under a later +// tenure's term. func (g Generation) Next(term uint64) Generation { if term > g.Term { return Generation{Term: term, Counter: 1} diff --git a/backend/bucketindex/groupsuccessor_test.go b/backend/bucketindex/groupsuccessor_test.go index 97ce1253..e6021e09 100644 --- a/backend/bucketindex/groupsuccessor_test.go +++ b/backend/bucketindex/groupsuccessor_test.go @@ -11,7 +11,7 @@ import ( ) func members3(name string, first uint64, claimed bucketindex.Interval, level uint32) []bucketindex.Entry { - c := bucketindex.Claim{Blocks: claimed, Group: bucketindex.Interval{Min: first, Max: first + 2}} + c := bucketindex.Claim{Blocks: claimed, Group: bucketindex.Range(0, first, first+2)} out := make([]bucketindex.Entry, 3) for i := range out { @@ -38,7 +38,7 @@ func TestSuccessorOfAGroupSubsumesEveryMember(t *testing.T) { t.Parallel() group := members3("f", 10, bucketindex.Blocks(1), 1) - succ := bucketindex.Entry{Prefix: "s", Blocks: bucketindex.Interval{Min: 1, Max: 2}, Level: 2} + succ := bucketindex.Entry{Prefix: "s", Blocks: bucketindex.Range(0, 1, 2), Level: 2} for _, m := range group { assert.True(t, succ.Supersedes(m), "%s holds %s's rows", succ.Prefix, m.Prefix) @@ -67,8 +67,8 @@ func TestSuccessorOfAGroupSubsumesEveryMember(t *testing.T) { t.Run("PartOfTheClaim", func(t *testing.T) { t.Parallel() - wide := members3("g", 10, bucketindex.Interval{Min: 1, Max: 2}, 1) - part := bucketindex.Entry{Prefix: "p", Blocks: bucketindex.Interval{Min: 2, Max: 3}, Level: 2} + wide := members3("g", 10, bucketindex.Range(0, 1, 2), 1) + part := bucketindex.Entry{Prefix: "p", Blocks: bucketindex.Range(0, 2, 3), Level: 2} assert.False(t, part.Supersedes(wide[0]), "block 1's rows are in the group and not in the part") assert.Empty(t, bucketindex.Subsumed(wide, []bucketindex.Entry{part})) }) diff --git a/backend/bucketindex/hole_test.go b/backend/bucketindex/hole_test.go index 3f440387..96294e93 100644 --- a/backend/bucketindex/hole_test.go +++ b/backend/bucketindex/hole_test.go @@ -12,7 +12,7 @@ import ( func lostWant() bucketindex.Want { return bucketindex.Want{ Prefix: "p/0000000002", - Blocks: bucketindex.Interval{Min: 2, Max: 2}, + Blocks: bucketindex.Range(0, 2, 2), Level: 1, MinTime: 100, MaxTime: 200, @@ -108,28 +108,28 @@ func TestRevokes(t *testing.T) { { name: "containing successor", live: bucketindex.Entry{ - Prefix: "p/0000000009", Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 2, + Prefix: "p/0000000009", Blocks: bucketindex.Range(0, 1, 4), Level: 2, }, want: true, }, { name: "overlapping at the same level", live: bucketindex.Entry{ - Prefix: "p/0000000009", Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 1, + Prefix: "p/0000000009", Blocks: bucketindex.Range(0, 1, 4), Level: 1, }, want: false, }, { name: "unrelated part", live: bucketindex.Entry{ - Prefix: "p/0000000009", Blocks: bucketindex.Interval{Min: 7, Max: 8}, Level: 3, + Prefix: "p/0000000009", Blocks: bucketindex.Range(0, 7, 8), Level: 3, }, want: false, }, { name: "another hole over the same blocks", live: bucketindex.Entry{ - Prefix: "p/0000000009", Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 2, Hole: true, + Prefix: "p/0000000009", Blocks: bucketindex.Range(0, 1, 4), Level: 2, Hole: true, }, want: false, }, @@ -159,10 +159,10 @@ func TestRevokesRejectsNonHole(t *testing.T) { func TestNextBlockCountsHoles(t *testing.T) { t.Parallel() - hole := bucketindex.Entry{Blocks: bucketindex.Interval{Min: 4, Max: 6}, Hole: true, Prefix: "h"} + hole := bucketindex.Entry{Blocks: bucketindex.Range(0, 4, 6), Hole: true, Prefix: "h"} ix := &bucketindex.Index{Entries: []bucketindex.Entry{hole}} - assert.EqualValues(t, 7, ix.NextBlock()) + assert.Equal(t, bucketindex.Block{N: 7}, ix.NextBlock(0)) } // TestHoleEncodingRoundTrip pins that the flag and the loss count survive the wire. @@ -200,7 +200,7 @@ func TestWantOfCarriesIdentity(t *testing.T) { ent := bucketindex.Entry{ Prefix: "p/0000000002", MinTime: 100, MaxTime: 200, - Blocks: bucketindex.Interval{Min: 2, Max: 2}, Level: 1, + Blocks: bucketindex.Range(0, 2, 2), Level: 1, } g := bucketindex.Generation{Term: 3, Counter: 4} diff --git a/backend/bucketindex/lineage.go b/backend/bucketindex/lineage.go index f1d2029a..b5dca08f 100644 --- a/backend/bucketindex/lineage.go +++ b/backend/bucketindex/lineage.go @@ -22,10 +22,21 @@ func LineageOf(entries []Entry) Lineage { // Subsumes reports whether e holds every row o holds, at a higher level: e covers o's blocks, or // the whole ancestry of the groups o's blocks belong to. +// +// Two parts with one identity written by different tenures — two owners that merged the same inputs +// either side of a handoff — hold the same rows, and the later tenure's copy subsumes the earlier. func (l Lineage) Subsumes(e, o Entry) bool { + if e.Term > o.Term && e.sameIdentity(o) { + return true + } + return e.Level > o.Level && l.holds(e.Blocks).Contains(o.Blocks) } +func (e Entry) sameIdentity(o Entry) bool { + return e.Level == o.Level && e.Blocks.Valid() && e.Blocks.Equal(o.Blocks) && e.Claim.Equal(o.Claim) +} + // Overlaps reports whether a and b may hold a row in common. // // Rows are shared only along descent: a block and a block derived from it overlap, as does a block @@ -101,16 +112,15 @@ func intersect(a, b Interval) Interval { return Interval{} } - ra, rb := a.runs(), b.runs() - var out []Gap - for i, j := 0, 0; i < len(ra) && j < len(rb); { - if lo, hi := max(ra[i].Min, rb[j].Min), min(ra[i].Max, rb[j].Max); lo <= hi { + for i, j := 0, 0; i < a.nruns() && j < b.nruns(); { + ra, rb := a.run(i), b.run(j) + if lo, hi := MaxBlock(ra.Min, rb.Min), minBlock(ra.Max, rb.Max); !hi.less(lo) { out = append(out, Gap{Min: lo, Max: hi}) } - if ra[i].Max < rb[j].Max { + if ra.Max.less(rb.Max) { i++ } else { j++ diff --git a/backend/bucketindex/lineage_test.go b/backend/bucketindex/lineage_test.go index 62fa3b9d..c01c339d 100644 --- a/backend/bucketindex/lineage_test.go +++ b/backend/bucketindex/lineage_test.go @@ -136,7 +136,7 @@ func (m *lineageModel) step() { slices.Sort(rows) m.rnd.Shuffle(len(rows), func(i, j int) { rows[i], rows[j] = rows[j], rows[i] }) - c := bucketindex.Claim{Blocks: union, Group: bucketindex.Interval{Min: m.next + 1, Max: m.next + uint64(k)}} + c := bucketindex.Claim{Blocks: union, Group: bucketindex.Range(0, m.next+1, m.next+uint64(k))} for i := range k { m.next++ p := modelPart{ diff --git a/backend/bucketindex/plan.go b/backend/bucketindex/plan.go new file mode 100644 index 00000000..b0f9712c --- /dev/null +++ b/backend/bucketindex/plan.go @@ -0,0 +1,186 @@ +package bucketindex + +// Plan is the identity a newly written part awaits until the commit that publishes it assigns one. A +// flush leaves Blocks unset, so the commit allocates a fresh block for it; a merge that writes one +// part inherits the set of blocks its inputs covered, which is what makes the output supersede them. +type Plan struct { + Blocks Interval + Claim Claim + // Group is set on the outputs of a merge that split, and shared by all of them: the commit + // allocates the whole run at once so the fragments can name each other. + Group *SplitGroup + Level uint32 +} + +// SplitGroup is the joint claim the outputs of one split merge carry. The ancestor blocks are known +// when the merge is planned; the group's own blocks are not, because allocation happens per commit +// attempt. +type SplitGroup struct { + Blocks Interval + N int +} + +// FlushPlan is the plan of a flush output: a fresh block at level 0. +func FlushPlan() *Plan { return &Plan{} } + +// PlanMerge returns the plans of the n outputs of a merge over src, whose identities are read from +// their Blocks, Claim and Level. +// +// A merge that writes one part does not allocate. The output covers exactly the blocks its inputs +// covered — a set, so a run that straddles a gap claims no block in it — one level above the +// deepest input, and that is precisely what makes supersession decidable from identity alone, so a +// repair can accept the successor of the part it wanted. +// +// A merge that splits its output cannot hand any fragment that set: none holds all of the data, so +// a successor claim would answer a want with a fraction of the part. The fragments take a fresh run +// of blocks instead and carry the inputs' set as a joint [Claim] over that run — which is what keeps +// the lineage: the claim is realized wherever every member of the run is present, and a later merge +// that consumes them all folds it back into an ordinary interval. +// +// A merge whose inputs all predate format v5 has no set to inherit; a fresh block is how such a +// part migrates, a merge being the only thing that rewrites it. A mixed merge — some inputs +// carrying an interval, some not — inherits the union of the ones that do. It cannot do better: a +// want naming a pre-v5 part records that part's unset interval, so no claim the output makes could +// contain it. +// +// One limit is deliberate: an output carries at most one unrealized claim, the widest, so a merge +// consuming fragments of two groups without completing either keeps the lineage of one. The other +// falls back to exact-prefix matching, which is where every split left it before claims existed. +func PlanMerge(src []Entry, n int) []*Plan { + var ( + union Interval + level uint32 + claims []Claim + ) + + for i := range src { + level = max(level, src[i].Level+1) + union = union.Union(src[i].Blocks) + + if src[i].Claim.Valid() { + claims = append(claims, src[i].Claim) + } + } + + union, claims = realizeClaims(union, claims) + + out := make([]*Plan, n) + + if n == 1 { + out[0] = &Plan{Blocks: union, Claim: widestClaim(claims), Level: level} + + return out + } + + group := &SplitGroup{Blocks: union, N: n} + for i := range out { + out[i] = &Plan{Group: group, Level: level} + } + + return out +} + +// realizeClaims folds into held every claim whose group is wholly inside it, repeating while that +// keeps realizing more — a group whose members were themselves split resolves only after the inner +// one has. It returns what is covered and the claims still owed a member. +func realizeClaims(held Interval, claims []Claim) (Interval, []Claim) { + for len(claims) > 0 { + rest := claims[:0] + + for _, c := range claims { + if held.Contains(c.Group) { + held = held.Union(c.Blocks) + + continue + } + + rest = append(rest, c) + } + + if len(rest) == len(claims) { + break + } + + claims = rest + } + + return held, claims +} + +func widestClaim(claims []Claim) Claim { + var best Claim + for _, c := range claims { + if c.Blocks.Len() > best.Blocks.Len() { + best = c + } + } + + return best +} + +// Allocator assigns the identities of one commit attempt, taking blocks upward from the one +// [Index.NextBlock] returned. A fresh one is made per attempt: a rival's entries are only known after +// a rebase, and reusing an attempt's allocation would hand a part a block the winner already claimed. +type Allocator struct { + next Block + groups map[*SplitGroup]*groupRun +} + +// groupRun is the run of blocks one commit attempt allocated to a split group, and how far into it +// the fragments assigned so far have got. +type groupRun struct { + run Interval + next Block +} + +// NewAllocator starts allocating at next. +func NewAllocator(next Block) *Allocator { + return &Allocator{next: next, groups: make(map[*SplitGroup]*groupRun)} +} + +// Term is the term every block this allocator hands out belongs to: the tenure the commit writes as. +func (a *Allocator) Term() uint64 { return a.next.Term } + +// Mark is the allocation high-water mark after everything assigned so far: the last block handed +// out, or the one below the first if none was. +func (a *Allocator) Mark() Block { return Block{Term: a.next.Term, N: a.next.N - 1} } + +// Assign turns p into a real identity. A plan that already names its blocks keeps them. +// +// A split group takes its whole run at once, before any of its fragments is numbered, so every +// fragment carries the same claim and can name the siblings a repair still has to fetch. +func (a *Allocator) Assign(p *Plan) (Interval, Claim) { + if p.Blocks.Valid() { + return p.Blocks, p.Claim + } + + if p.Group == nil { + b := a.take(1) + + return Interval{Min: b, Max: b}, p.Claim + } + + g, ok := a.groups[p.Group] + if !ok { + lo := a.take(uint64(p.Group.N)) + g = &groupRun{run: Interval{Min: lo, Max: Block{Term: lo.Term, N: lo.N + uint64(p.Group.N) - 1}}, next: lo} + a.groups[p.Group] = g + } + + b := g.next + g.next.N++ + + claim := Claim{Blocks: p.Group.Blocks, Group: g.run} + if !claim.Valid() { + return Interval{Min: b, Max: b}, Claim{} + } + + return Interval{Min: b, Max: b}, claim +} + +func (a *Allocator) take(n uint64) Block { + b := a.next + a.next.N += n + + return b +} diff --git a/backend/bucketindex/tenure.go b/backend/bucketindex/tenure.go new file mode 100644 index 00000000..220a0928 --- /dev/null +++ b/backend/bucketindex/tenure.go @@ -0,0 +1,29 @@ +package bucketindex + +import "github.com/go-faster/errors" + +// ErrSuperseded is returned (wrapped) by a clustered writer that refuses to commit because it can no +// longer prove it writes as the shard's current tenure. Whatever the refused commit would have +// published is left unreferenced, for the orphan sweep to reclaim. +var ErrSuperseded = errors.New("bucketindex: commit fenced: not the shard's current tenure") + +// CheckTenure is the commit fence a clustered writer runs before each conditional index write. +// stamped is the ownership term the operation began under, current the term the writer holds now +// (0 for none, including a claim it can no longer prove), and indexed the term of the index the +// commit is built on. +// +// Each refusal closes a way for a writer that has been displaced to commit anyway: a claim that +// lapsed or was fenced, a tenure that ended and restarted while the operation was in flight, and an +// index a later tenure already wrote, which a rebase reveals and a commit must never adopt. +func CheckTenure(stamped, current, indexed uint64) error { + switch { + case current == 0: + return errors.Wrap(ErrSuperseded, "no claim on the shard") + case stamped != current: + return errors.Wrapf(ErrSuperseded, "operation began under term %d, the claim is now term %d", stamped, current) + case indexed > current: + return errors.Wrapf(ErrSuperseded, "index committed under term %d, above this writer's %d", indexed, current) + default: + return nil + } +} diff --git a/backend/bucketindex/tenure_test.go b/backend/bucketindex/tenure_test.go new file mode 100644 index 00000000..7521d8bf --- /dev/null +++ b/backend/bucketindex/tenure_test.go @@ -0,0 +1,78 @@ +package bucketindex_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/oteldb/storage/backend/bucketindex" +) + +func TestCheckTenure(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + stamped, current, indexed uint64 + ok bool + }{ + {"current tenure", 5, 5, 5, true}, + {"first commit of a new tenure", 5, 5, 3, true}, + {"legacy index", 5, 5, 0, true}, + {"no claim", 5, 0, 3, false}, + {"never held", 0, 0, 0, false}, + {"tenure restarted mid-operation", 5, 7, 5, false}, + {"a later tenure wrote the index", 5, 5, 6, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + err := bucketindex.CheckTenure(tc.stamped, tc.current, tc.indexed) + if tc.ok { + assert.NoError(t, err) + + return + } + + assert.ErrorIs(t, err, bucketindex.ErrSuperseded) + }) + } +} + +func TestAllocatorAssignsUnderItsTerm(t *testing.T) { + t.Parallel() + + a := bucketindex.NewAllocator(bucketindex.Block{Term: 9, N: 4}) + assert.EqualValues(t, 9, a.Term()) + assert.Equal(t, bucketindex.Block{Term: 9, N: 3}, a.Mark(), "nothing assigned yet") + + flush, _ := a.Assign(bucketindex.FlushPlan()) + assert.Equal(t, bucketindex.TermBlocks(9, 4), flush) + + src := []bucketindex.Entry{ + {Blocks: bucketindex.TermBlocks(2, 1)}, + {Blocks: flush, Level: 1}, + } + + single := bucketindex.PlanMerge(src, 1) + require.Len(t, single, 1) + blocks, claim := a.Assign(single[0]) + assert.Equal(t, bucketindex.TermBlocks(2, 1).Union(flush), blocks, "a whole merge inherits across tenures") + assert.False(t, claim.Valid()) + assert.EqualValues(t, 2, single[0].Level) + + split := bucketindex.PlanMerge(src, 3) + var group bucketindex.Claim + + for i, p := range split { + b, c := a.Assign(p) + assert.Equal(t, bucketindex.TermBlocks(9, uint64(5+i)), b, "fragment %d", i) + + group = c + } + + assert.Equal(t, bucketindex.Claim{Blocks: blocks, Group: bucketindex.Range(9, 5, 7)}, group) + assert.Equal(t, bucketindex.Block{Term: 9, N: 7}, a.Mark()) +} diff --git a/backend/bucketindex/testdata/fuzz/FuzzRoundTrip/5b43b470a18e2e78 b/backend/bucketindex/testdata/fuzz/FuzzRoundTrip/5b43b470a18e2e78 index 318b64f3..a4120d77 100644 --- a/backend/bucketindex/testdata/fuzz/FuzzRoundTrip/5b43b470a18e2e78 +++ b/backend/bucketindex/testdata/fuzz/FuzzRoundTrip/5b43b470a18e2e78 @@ -4,3 +4,4 @@ uint32(44) uint64(0) uint64(1) uint64(0) +uint64(0) diff --git a/backend/bucketindex/want.go b/backend/bucketindex/want.go index 062df7e6..d39cedb7 100644 --- a/backend/bucketindex/want.go +++ b/backend/bucketindex/want.go @@ -42,6 +42,7 @@ type Want struct { Blocks Interval Claim Claim Level uint32 + Term uint64 MinTime, MaxTime int64 Generation Generation } @@ -49,7 +50,7 @@ type Want struct { // WantOf is the repair obligation a lost entry owes, discovered at generation g. func WantOf(e Entry, g Generation) Want { return Want{ - Prefix: e.Prefix, Blocks: e.Blocks, Claim: e.Claim, Level: e.Level, + Prefix: e.Prefix, Blocks: e.Blocks, Claim: e.Claim, Level: e.Level, Term: e.Term, MinTime: e.MinTime, MaxTime: e.MaxTime, Generation: g, } } @@ -58,7 +59,7 @@ func WantOf(e Entry, g Generation) Want { func (w Want) Entry() Entry { return Entry{ Prefix: w.Prefix, MinTime: w.MinTime, MaxTime: w.MaxTime, - Blocks: w.Blocks, Claim: w.Claim, Level: w.Level, + Blocks: w.Blocks, Claim: w.Claim, Level: w.Level, Term: w.Term, } } diff --git a/backend/bucketindex/want_test.go b/backend/bucketindex/want_test.go index 46b20a2d..4e82aa53 100644 --- a/backend/bucketindex/want_test.go +++ b/backend/bucketindex/want_test.go @@ -16,9 +16,9 @@ func TestRecordWantKeepsSortedAndReplaces(t *testing.T) { t.Parallel() var ix bucketindex.Index - ix.RecordWant(bucketindex.Want{Prefix: "c", Blocks: bucketindex.Interval{Min: 3, Max: 3}}) - ix.RecordWant(bucketindex.Want{Prefix: "a", Blocks: bucketindex.Interval{Min: 1, Max: 1}}) - ix.RecordWant(bucketindex.Want{Prefix: "b", Blocks: bucketindex.Interval{Min: 2, Max: 2}}) + ix.RecordWant(bucketindex.Want{Prefix: "c", Blocks: bucketindex.Range(0, 3, 3)}) + ix.RecordWant(bucketindex.Want{Prefix: "a", Blocks: bucketindex.Range(0, 1, 1)}) + ix.RecordWant(bucketindex.Want{Prefix: "b", Blocks: bucketindex.Range(0, 2, 2)}) prefixes := []string{ix.Wanted[0].Prefix, ix.Wanted[1].Prefix, ix.Wanted[2].Prefix} assert.Equal(t, []string{"a", "b", "c"}, prefixes) @@ -47,7 +47,7 @@ func TestWants(t *testing.T) { var ix bucketindex.Index assert.Empty(t, ix.Wants()) - w := bucketindex.Want{Prefix: "p", Blocks: bucketindex.Interval{Min: 4, Max: 4}} + w := bucketindex.Want{Prefix: "p", Blocks: bucketindex.Range(0, 4, 4)} ix.RecordWant(w) assert.Equal(t, map[string]bucketindex.Want{"p": w}, ix.Wants()) } @@ -56,13 +56,13 @@ func TestTrimWantsDropsDischarged(t *testing.T) { t.Parallel() wants := []bucketindex.Want{ - {Prefix: "back", Blocks: bucketindex.Interval{Min: 1, Max: 1}}, - {Prefix: "merged", Blocks: bucketindex.Interval{Min: 2, Max: 2}}, - {Prefix: "still-gone", Blocks: bucketindex.Interval{Min: 9, Max: 9}}, + {Prefix: "back", Blocks: bucketindex.Range(0, 1, 1)}, + {Prefix: "merged", Blocks: bucketindex.Range(0, 2, 2)}, + {Prefix: "still-gone", Blocks: bucketindex.Range(0, 9, 9)}, } live := []bucketindex.Entry{ - {Prefix: "back", Blocks: bucketindex.Interval{Min: 1, Max: 1}}, - {Prefix: "successor", Blocks: bucketindex.Interval{Min: 2, Max: 5}, Level: 1}, + {Prefix: "back", Blocks: bucketindex.Range(0, 1, 1)}, + {Prefix: "successor", Blocks: bucketindex.Range(0, 2, 5), Level: 1}, } kept := bucketindex.TrimWants(wants, live) diff --git a/cluster/ARCH.md b/cluster/ARCH.md index 11fdae5c..4508c3b7 100644 --- a/cluster/ARCH.md +++ b/cluster/ARCH.md @@ -72,6 +72,13 @@ therefore drops such a key under a value+lease guard and recreates it under the also makes the restart a real new tenure: the recreated claim takes a higher term, so the incarnation's writes outrank the dead one's. +The term is also what scopes a tenure's **part identity** and **fences its commits**: blocks are +allocated as `(term, n)`, and an engine refuses to commit once `Ownership.Term` no longer reports the +term its operation began under, or once the index it builds on carries a higher one +(`backend/ARCH.md`, *Part identity*). A handoff therefore never stalls a shard: the new owner flushes +and merges with the freshest view it has. The cost is duplicate rows, never a collision — an old +owner's last flush that no replica mirrored is re-flushed by the new owner from its replica head. + Tests boot etcd through **`cluster/etcd/etcdtest`**, which binds its listeners to port 0 and reads the client port back from the socket. Probing a free port and releasing it for etcd to bind is racy: anything, including etcd clients' own ephemeral ports, can take it in between. Only diff --git a/cluster/partsync/deferredsync_test.go b/cluster/partsync/deferredsync_test.go index 7647540d..f41f0f2c 100644 --- a/cluster/partsync/deferredsync_test.go +++ b/cluster/partsync/deferredsync_test.go @@ -62,7 +62,7 @@ func TestFetchWantsSyncsTheCopiedPart(t *testing.T) { be := backend.Memory() ix := &bucketindex.Index{} - merged := writeBlockPart(t, be, ix, prefix, "0100", bucketindex.Interval{Min: 1, Max: 4}, 1) + merged := writeBlockPart(t, be, ix, prefix, "0100", bucketindex.Range(0, 1, 4), 1) saveIndex(t, be, prefix, ix) peer := servePeer(t, be, prefix, probeOpts{}) diff --git a/cluster/partsync/omitted_test.go b/cluster/partsync/omitted_test.go index 2cb6964a..0af2f1dd 100644 --- a/cluster/partsync/omitted_test.go +++ b/cluster/partsync/omitted_test.go @@ -117,8 +117,8 @@ func syncOmissionAfterMerge(t *testing.T, tombstone bool) { ix := &bucketindex.Index{Generation: gen(1, 5)} writePart(t, owner, ix, omitPrefix, 1, 100, 150) writePart(t, owner, ix, omitPrefix, 2, 200, 250) - ix.Entries[0].Blocks, ix.Entries[0].Level = bucketindex.Interval{Min: 1, Max: 1}, 0 - ix.Entries[1].Blocks, ix.Entries[1].Level = bucketindex.Interval{Min: 2, Max: 2}, 0 + ix.Entries[0].Blocks, ix.Entries[0].Level = bucketindex.Range(0, 1, 1), 0 + ix.Entries[1].Blocks, ix.Entries[1].Level = bucketindex.Range(0, 2, 2), 0 saveIndex(t, owner, omitPrefix, ix) addr := serve(t, owner) @@ -136,7 +136,7 @@ func syncOmissionAfterMerge(t *testing.T, tombstone bool) { merged := &bucketindex.Index{Generation: gen(1, 6)} writePart(t, owner, merged, omitPrefix, 3, 100, 250) - merged.Entries[0].Blocks, merged.Entries[0].Level = bucketindex.Interval{Min: 1, Max: 2}, 1 + merged.Entries[0].Blocks, merged.Entries[0].Level = bucketindex.Range(0, 1, 2), 1 if tombstone { for _, seq := range []int{1, 2} { diff --git a/cluster/partsync/repair_test.go b/cluster/partsync/repair_test.go index 595effb2..16f805a6 100644 --- a/cluster/partsync/repair_test.go +++ b/cluster/partsync/repair_test.go @@ -36,13 +36,13 @@ func TestFetchWantExactPart(t *testing.T) { owner, damaged := backend.Memory(), backend.Memory() ix := &bucketindex.Index{} - part := writeBlockPart(t, owner, ix, "default/logs", "0001", bucketindex.Interval{Min: 1, Max: 1}, 0) + part := writeBlockPart(t, owner, ix, "default/logs", "0001", bucketindex.Range(0, 1, 1), 0) saveIndex(t, owner, "default/logs", ix) s := partsync.New(damaged, &partsync.Client{}) ent, ok, err := s.FetchWant(ctx, "default/logs", []string{serve(t, owner)}, - bucketindex.Want{Prefix: part, Blocks: bucketindex.Interval{Min: 1, Max: 1}}) + bucketindex.Want{Prefix: part, Blocks: bucketindex.Range(0, 1, 1)}) require.NoError(t, err) require.True(t, ok) assert.Equal(t, part, ent.Prefix) @@ -64,13 +64,13 @@ func TestFetchWantPrefersContainingSuccessor(t *testing.T) { // The wanted part is gone from the peer too — merged into a level-1 successor covering it. ix := &bucketindex.Index{} - merged := writeBlockPart(t, owner, ix, "default/logs", "0100", bucketindex.Interval{Min: 1, Max: 4}, 1) + merged := writeBlockPart(t, owner, ix, "default/logs", "0100", bucketindex.Range(0, 1, 4), 1) saveIndex(t, owner, "default/logs", ix) s := partsync.New(damaged, &partsync.Client{}) ent, ok, err := s.FetchWant(ctx, "default/logs", []string{serve(t, owner)}, - bucketindex.Want{Prefix: "default/logs/0002", Blocks: bucketindex.Interval{Min: 2, Max: 2}}) + bucketindex.Want{Prefix: "default/logs/0002", Blocks: bucketindex.Range(0, 2, 2)}) require.NoError(t, err) require.True(t, ok) assert.Equal(t, merged, ent.Prefix, "the successor containing the want, not the vanished prefix") @@ -87,17 +87,17 @@ func TestFetchWantPicksWidestAcrossPeers(t *testing.T) { narrow, wide, damaged := backend.Memory(), backend.Memory(), backend.Memory() nix := &bucketindex.Index{} - writeBlockPart(t, narrow, nix, "default/logs", "0002", bucketindex.Interval{Min: 2, Max: 2}, 0) + writeBlockPart(t, narrow, nix, "default/logs", "0002", bucketindex.Range(0, 2, 2), 0) saveIndex(t, narrow, "default/logs", nix) wix := &bucketindex.Index{} - big := writeBlockPart(t, wide, wix, "default/logs", "0200", bucketindex.Interval{Min: 1, Max: 8}, 2) + big := writeBlockPart(t, wide, wix, "default/logs", "0200", bucketindex.Range(0, 1, 8), 2) saveIndex(t, wide, "default/logs", wix) s := partsync.New(damaged, &partsync.Client{}) ent, ok, err := s.FetchWant(ctx, "default/logs", []string{serve(t, narrow), serve(t, wide)}, - bucketindex.Want{Prefix: "default/logs/0002", Blocks: bucketindex.Interval{Min: 2, Max: 2}}) + bucketindex.Want{Prefix: "default/logs/0002", Blocks: bucketindex.Range(0, 2, 2)}) require.NoError(t, err) require.True(t, ok) assert.Equal(t, big, ent.Prefix) @@ -110,13 +110,13 @@ func TestFetchWantNoPeerHasIt(t *testing.T) { owner, damaged := backend.Memory(), backend.Memory() ix := &bucketindex.Index{} - writeBlockPart(t, owner, ix, "default/logs", "0009", bucketindex.Interval{Min: 9, Max: 9}, 0) + writeBlockPart(t, owner, ix, "default/logs", "0009", bucketindex.Range(0, 9, 9), 0) saveIndex(t, owner, "default/logs", ix) s := partsync.New(damaged, &partsync.Client{}) _, ok, err := s.FetchWant(ctx, "default/logs", []string{serve(t, owner)}, - bucketindex.Want{Prefix: "default/logs/0002", Blocks: bucketindex.Interval{Min: 2, Max: 2}}) + bucketindex.Want{Prefix: "default/logs/0002", Blocks: bucketindex.Range(0, 2, 2)}) require.NoError(t, err, "every peer answered, so absence is definitive rather than an error") assert.False(t, ok) } @@ -128,7 +128,7 @@ func TestFetchWantUnreachablePeerIsTransient(t *testing.T) { s := partsync.New(backend.Memory(), &partsync.Client{}) _, ok, err := s.FetchWant(ctx, "default/logs", []string{"127.0.0.1:1"}, - bucketindex.Want{Prefix: "default/logs/0002", Blocks: bucketindex.Interval{Min: 2, Max: 2}}) + bucketindex.Want{Prefix: "default/logs/0002", Blocks: bucketindex.Range(0, 2, 2)}) require.Error(t, err, "a peer we could not ask is not evidence the part is gone") assert.False(t, ok) } @@ -153,14 +153,14 @@ func TestFetchWantPartialPeerObjectsFail(t *testing.T) { ix := &bucketindex.Index{} ix.Add(bucketindex.Entry{ Prefix: "default/logs/0001", MinTime: 1, MaxTime: 2, - Blocks: bucketindex.Interval{Min: 1, Max: 1}, + Blocks: bucketindex.Range(0, 1, 1), }) saveIndex(t, owner, "default/logs", ix) s := partsync.New(damaged, &partsync.Client{}) _, ok, err := s.FetchWant(ctx, "default/logs", []string{serve(t, owner)}, - bucketindex.Want{Prefix: "default/logs/0001", Blocks: bucketindex.Interval{Min: 1, Max: 1}}) + bucketindex.Want{Prefix: "default/logs/0001", Blocks: bucketindex.Range(0, 1, 1)}) require.Error(t, err) assert.False(t, ok) } diff --git a/cluster/partsync/repairlisting_test.go b/cluster/partsync/repairlisting_test.go index ccc868b0..0c9a9974 100644 --- a/cluster/partsync/repairlisting_test.go +++ b/cluster/partsync/repairlisting_test.go @@ -22,7 +22,7 @@ func stageHeldButUnindexed(t *testing.T, be backend.Backend, prefix string, clai t.Helper() staging := &bucketindex.Index{} - writeBlockPart(t, be, staging, prefix, "0001", bucketindex.Interval{Min: 1, Max: 1}, 0) + writeBlockPart(t, be, staging, prefix, "0001", bucketindex.Range(0, 1, 1), 0) ent := staging.Entries[0] ix := &bucketindex.Index{Generation: gen(1, 2)} @@ -126,7 +126,7 @@ func TestFetchWantsListsEachPeerOncePerCycle(t *testing.T) { staging := &bucketindex.Index{} for j, name := range names { - writeBlockPart(t, be, staging, prefix, name, bucketindex.Interval{Min: uint64(j + 1), Max: uint64(j + 1)}, 0) + writeBlockPart(t, be, staging, prefix, name, bucketindex.Range(0, uint64(j+1), uint64(j+1)), 0) wants[j] = bucketindex.WantOf(staging.Entries[j], gen(0, 0)) } diff --git a/cluster/partsync/repairselect_test.go b/cluster/partsync/repairselect_test.go index a6e926da..7d6ac03a 100644 --- a/cluster/partsync/repairselect_test.go +++ b/cluster/partsync/repairselect_test.go @@ -118,7 +118,7 @@ func (p *probePeer) reset() { func wantAt(prefix string, block uint64) bucketindex.Want { return bucketindex.Want{ Prefix: prefix, - Blocks: bucketindex.Interval{Min: block, Max: block}, + Blocks: bucketindex.Range(0, block, block), } } @@ -136,7 +136,7 @@ func TestFetchWantsReadsEachIndexOncePerCycle(t *testing.T) { for i := range peers { be := backend.Memory() ix := &bucketindex.Index{} - writeBlockPart(t, be, ix, prefix, "0100", bucketindex.Interval{Min: 1, Max: 8}, 1) + writeBlockPart(t, be, ix, prefix, "0100", bucketindex.Range(0, 1, 8), 1) saveIndex(t, be, prefix, ix) peers[i] = servePeer(t, be, prefix, probeOpts{}) @@ -180,7 +180,7 @@ func TestFetchWantsCopiesASharedSuccessorOnce(t *testing.T) { be := backend.Memory() ix := &bucketindex.Index{} - merged := writeBlockPart(t, be, ix, prefix, "0100", bucketindex.Interval{Min: 1, Max: 4}, 1) + merged := writeBlockPart(t, be, ix, prefix, "0100", bucketindex.Range(0, 1, 4), 1) saveIndex(t, be, prefix, ix) peer := servePeer(t, be, prefix, probeOpts{}) @@ -221,12 +221,12 @@ func TestFetchWantsSelectionIsOrderIndependent(t *testing.T) { for seed := range uint64(4) { narrow := backend.Memory() nix := &bucketindex.Index{} - writeBlockPart(t, narrow, nix, prefix, "0002", bucketindex.Interval{Min: 2, Max: 2}, 0) + writeBlockPart(t, narrow, nix, prefix, "0002", bucketindex.Range(0, 2, 2), 0) saveIndex(t, narrow, prefix, nix) wide := backend.Memory() wix := &bucketindex.Index{} - big := writeBlockPart(t, wide, wix, prefix, "0200", bucketindex.Interval{Min: 1, Max: 8}, 2) + big := writeBlockPart(t, wide, wix, prefix, "0200", bucketindex.Range(0, 1, 8), 2) saveIndex(t, wide, prefix, wix) fast := servePeer(t, narrow, prefix, probeOpts{}) @@ -306,7 +306,7 @@ func stageIdenticalPeers(t *testing.T, prefix string, n int) (peers []*probePeer for i := range peers { be := backend.Memory() ix := &bucketindex.Index{} - part = writeBlockPart(t, be, ix, prefix, "0100", bucketindex.Interval{Min: 1, Max: 4}, 1) + part = writeBlockPart(t, be, ix, prefix, "0100", bucketindex.Range(0, 1, 4), 1) saveIndex(t, be, prefix, ix) peers[i] = servePeer(t, be, prefix, probeOpts{}) @@ -331,7 +331,7 @@ func TestFetchWantsAggregatesPeerErrors(t *testing.T) { for i := range 4 { be := backend.Memory() ix := &bucketindex.Index{} - writeBlockPart(t, be, ix, prefix, "0009", bucketindex.Interval{Min: 9, Max: 9}, 0) + writeBlockPart(t, be, ix, prefix, "0009", bucketindex.Range(0, 9, 9), 0) saveIndex(t, be, prefix, ix) addrs = append(addrs, servePeer(t, be, prefix, probeOpts{failIndex: i == 0}).addr) diff --git a/cluster/partsync/wants_internal_test.go b/cluster/partsync/wants_internal_test.go index ef6f1935..00687a1f 100644 --- a/cluster/partsync/wants_internal_test.go +++ b/cluster/partsync/wants_internal_test.go @@ -28,8 +28,8 @@ func TestUnaccountedEntriesGating(t *testing.T) { t.Parallel() g := bucketindex.Generation{Term: 1, Counter: 7} - mine := wantEntry("p/mine", bucketindex.Interval{Min: 5, Max: 5}, 500, 600) - theirs := wantEntry("p/theirs", bucketindex.Interval{Min: 6, Max: 6}, 700, 800) + mine := wantEntry("p/mine", bucketindex.Range(0, 5, 5), 500, 600) + theirs := wantEntry("p/theirs", bucketindex.Range(0, 6, 6), 700, 800) for name, tc := range map[string]struct { local *bucketindex.Index @@ -48,7 +48,7 @@ func TestUnaccountedEntriesGating(t *testing.T) { "older than everything kept is not wanted": { local: wantIndex(g, mine), peer: wantIndex(g, mine, - wantEntry("p/ancient", bucketindex.Interval{Min: 6, Max: 6}, 1, 2)), + wantEntry("p/ancient", bucketindex.Range(0, 6, 6), 1, 2)), }, "a pre-tombstone local index wants nothing": { local: wantIndex(bucketindex.Generation{}, mine), @@ -81,8 +81,8 @@ func TestWantsNeedConfirmation(t *testing.T) { t.Parallel() g := bucketindex.Generation{Term: 1, Counter: 7} - mine := wantEntry("p/mine", bucketindex.Interval{Min: 5, Max: 5}, 500, 600) - theirs := wantEntry("p/theirs", bucketindex.Interval{Min: 6, Max: 6}, 700, 800) + mine := wantEntry("p/mine", bucketindex.Range(0, 5, 5), 500, 600) + theirs := wantEntry("p/theirs", bucketindex.Range(0, 6, 6), 700, 800) s := New(nil, nil) local, peer := wantIndex(g, mine), wantIndex(g, mine, theirs) diff --git a/cluster_recovered_seams_test.go b/cluster_recovered_seams_test.go index b61204b0..eec06442 100644 --- a/cluster_recovered_seams_test.go +++ b/cluster_recovered_seams_test.go @@ -135,23 +135,36 @@ func prefixesOf(parts []PartInfo) []string { return out } -// restartLosing closes node-a, lets drop rewrite what it left on disk, and reopens it with the -// maintenance loop off, so the only thing that can bring the part back is the engine's own repair -// pass — a partsync mirror would restore the objects and hide a missing seam. -func (p *recoveredPair) restartLosing(t *testing.T, drop func(be *file.File)) *Storage { +// restartLosing closes the shard's compaction owner, lets drop rewrite what it left on disk, and +// reopens it with the maintenance loop off, so the only thing that can bring the part back is the +// engine's own repair pass — a partsync mirror would restore the objects and hide a missing seam. It +// has to be the owner: an engine commits only under the shard's claim, which the returned claim +// func takes back once the peer has let it go. +func (p *recoveredPair) restartLosing(t *testing.T, drop func(be *file.File)) (*Storage, signal.TenantID, func() bool) { t.Helper() - require.NoError(t, p.nodes["node-a"].Close(context.Background())) + shard := shardKeyOf("default", 0, p.nodes["node-a"].cluster.shardCount()) + primary, ok := p.nodes["node-a"].cluster.membership.Ring().Primary([]byte(shard)) + require.True(t, ok) - be, err := file.New(p.dirs["node-a"]) + id := primary.ID + require.NoError(t, p.nodes[id].Close(context.Background())) + + be, err := file.New(p.dirs[id]) require.NoError(t, err) drop(be) - a := p.open(t, "node-a", WithFlushInterval(-1)) - p.nodes["node-a"] = a + a := p.open(t, id, WithFlushInterval(-1)) + p.nodes[id] = a awaitMembership(t, p.nodes) - return a + claim := func() bool { + _, owned := a.ownedTenants(context.Background(), map[signal.TenantID]struct{}{shard: {}})[shard] + + return owned + } + + return a, shard, claim } func dropPartObjects(ctx context.Context, t *testing.T, be *file.File, prefix string) { @@ -176,14 +189,17 @@ func TestRecoveredEngineRepairsFromPeer(t *testing.T) { p := newRecoveredPair(t) lost := p.sharedPart(t, signal.Log) - a := p.restartLosing(t, func(be *file.File) { dropPartObjects(ctx, t, be, lost.Prefix) }) + a, shard, claim := p.restartLosing(t, func(be *file.File) { dropPartObjects(ctx, t, be, lost.Prefix) }) - shard := shardKeyOf("default", 0, a.cluster.shardCount()) eng, ok := a.lookupRecordEngine(signal.Log, shard) require.True(t, ok, "the engine exists from recovery, before any write") require.Positive(t, eng.Stats().WantedParts, "recovery recorded the loss") require.Eventually(t, func() bool { + if !claim() { + return false + } + _ = eng.Merge(ctx, 0) return eng.Stats().WantedParts == 0 @@ -204,7 +220,7 @@ func TestRecoveredEngineRepairsAdoptedWant(t *testing.T) { p := newRecoveredPair(t) lost := p.sharedPart(t, signal.Metric) - a := p.restartLosing(t, func(be *file.File) { + a, shard, claim := p.restartLosing(t, func(be *file.File) { key := path.Dir(lost.Prefix) + "/" + bucketindex.Object ix, err := bucketindex.Load(ctx, be, key) @@ -215,7 +231,6 @@ func TestRecoveredEngineRepairsAdoptedWant(t *testing.T) { dropPartObjects(ctx, t, be, lost.Prefix) }) - shard := shardKeyOf("default", 0, a.cluster.shardCount()) eng, ok := a.lookupEngine(shard) require.True(t, ok, "the engine exists from recovery, before any write") require.False(t, eng.HasWants(), "the local index never named the part, so nothing states it is owed") @@ -224,6 +239,10 @@ func TestRecoveredEngineRepairsAdoptedWant(t *testing.T) { require.True(t, eng.HasWants()) require.Eventually(t, func() bool { + if !claim() { + return false + } + _ = eng.Merge(ctx, 0) return !eng.HasWants() diff --git a/cluster_tenure_test.go b/cluster_tenure_test.go index 280a6755..b7621439 100644 --- a/cluster_tenure_test.go +++ b/cluster_tenure_test.go @@ -128,8 +128,8 @@ func TestClusterHandoffUnsyncedFlushReadsOnce(t *testing.T) { }) t.Run("logs", func(t *testing.T) { - reproduce.Unfixed(t, 725, "the new owner re-flushes head rows the old owner's unsynced part holds, and once "+ - "the old owner is back both parts stay live and merge into one part holding every record twice") + reproduce.Unfixed(t, 572, "the old owner's unsynced flush is the accepted cost of a handoff that never "+ + "stalls: the new owner re-flushes the same head rows under its own term, and nothing dedups the records") for id, s := range nodes { got, err := fetchLogs(ctx, s, 0, 1<<62) diff --git a/engine/ARCH.md b/engine/ARCH.md index d14a223a..5e8f59fb 100644 --- a/engine/ARCH.md +++ b/engine/ARCH.md @@ -149,9 +149,17 @@ The opens sit on the commit path, which is where the cost is. It is bounded: han across the retry loop, and an entry that cannot be opened — a rival merged it away in between — is left out of the readable set but kept in the index, since only its writer knows whether it is live. +**An adopted part this engine's own parts subsume is retired instead** (`dropSubsumedForeignLocked`): +the same merge a displaced tenure committed just before this one, resolved to the later tenure, or +an input a merge here has since consumed. It is left out of the commit and tombstoned, since +carrying it forward keeps its rows live twice; its objects are the orphan sweep's. + ### Block identity is allocated by the commit that publishes the part -A flush output commits `{n}` at level 0, `n` from `bucketindex.Index.NextBlock`. A merge that writes +A flush output commits `{b}` at level 0, `b` from `bucketindex.Index.NextBlock` under the term the +commit writes as, so every block is scoped to its tenure (`backend/ARCH.md`, "Part identity"). The +planning and allocation (`bucketindex.Plan`, `PlanMerge`, `Allocator`) are shared with +`recordengine`; each engine keeps only the per-attempt bookkeeping over its own parts. A merge that writes **one** part commits the union of the block sets its inputs covered, at `max(input level) + 1`, and allocates nothing: the merged part covers exactly the blocks its inputs covered — a set, so a run that straddles a gap claims no block inside it — and that is what makes `Entry.Supersedes`, and so a @@ -281,6 +289,18 @@ verifier reclaimed objects from the directory it was pointed at. It backs `stora which then keeps the guarantee at the facade for the life of the handle. +## A clustered commit is fenced by tenure + +Before every CAS attempt, `commitIndexLocked` runs `bucketindex.CheckTenure` (`tenureLocked`) over +the term the operation began with, the term the engine holds now, and the term of the index it +builds on (`generation.Term`, which a load or a rebase raises). A writer with no `Config.Term` is +not fenced. A merge stamps its term when it picks its inputs: one whose tenure ended and restarted +while it ran chose them from a view the intervening tenure may have merged differently. A flush, +a retention drop and a repair commit take the term at commit instead, since what they publish is +this node's own head or a part it fetched, valid under any tenure the node holds. A refused flush +keeps its part as a carried, uncommitted part, like any failed commit; a refused merge rolls back +and leaves its output to the sweep. + ## A failed load changes nothing, and fences every commit A load is all-or-nothing. `readIndexLocked` reads the index and opens every part it names into diff --git a/engine/blockid.go b/engine/blockid.go index b3d8e666..8eba82df 100644 --- a/engine/blockid.go +++ b/engine/blockid.go @@ -6,30 +6,9 @@ import ( "github.com/oteldb/storage/backend/bucketindex" ) -// A part's identity in block-number space is assigned by the commit that publishes it, out of the -// index that commit is building. Allocation is the shard owner's alone and needs no coordination: -// the CAS that adds the part is what claims the number, so a writer that loses the race re-reads -// and allocates above the winner. - -// blockPlan is the identity a newly written part awaits. A flush leaves blocks unset, so the -// commit allocates a fresh number for it; a merge that writes one part inherits the set of blocks -// its inputs covered, which is what makes the output supersede them. -type blockPlan struct { - blocks bucketindex.Interval - claim bucketindex.Claim - // group is set on the outputs of a merge that split, and shared by all of them: the commit - // allocates the whole run at once so the fragments can name each other. - group *splitGroup - level uint32 -} - -// splitGroup is the joint claim the outputs of one split merge carry. The ancestor blocks are -// known when the merge is planned; the group's own blocks are not, because allocation happens per -// commit attempt — see [Engine.nextIndexLocked]. -type splitGroup struct { - blocks bucketindex.Interval - n int -} +// A part's identity in block space is assigned by the commit that publishes it, out of the index +// that commit is building, under the tenure it writes as: see [bucketindex.Plan] and +// [bucketindex.Allocator], which both engines share. // blockAssignment is an identity [Engine.nextIndexLocked] chose while building one candidate // index. It is written onto the part only by the commit that lands: a part numbered before its @@ -40,99 +19,23 @@ type blockAssignment struct { blocks bucketindex.Interval claim bucketindex.Claim level uint32 + term uint64 } // planFlushBlocks marks p as a flush output: a fresh block at level 0. -func planFlushBlocks(p *part) { p.pending = &blockPlan{} } +func planFlushBlocks(p *part) { p.pending = bucketindex.FlushPlan() } -// planMergeBlocks stamps the identity the outputs of a merge over src claim. -// -// A merge that writes one part does not allocate. The output covers exactly the blocks its inputs -// covered — a set, so a run that straddles a gap claims no block in it — one level above the -// deepest input, and that is precisely what makes supersession decidable from identity alone, so a -// repair can accept the successor of the part it wanted. -// -// A merge that splits its output cannot hand any fragment that set: none holds all of the data, so -// a successor claim would answer a want with a fraction of the part. The fragments take a fresh run -// of blocks instead and carry the inputs' set as a joint [bucketindex.Claim] over that run — which -// is what keeps the lineage: the claim is realized wherever every member of the run is present, and -// a later merge that consumes them all folds it back into an ordinary interval. -// -// A merge whose inputs all predate format v5 has no set to inherit; a fresh block is how such a -// part migrates, a merge being the only thing that rewrites it. A mixed merge — some inputs -// carrying an interval, some not — inherits the union of the ones that do. It cannot do better: a -// want naming a pre-v5 part records that part's unset interval, so no claim the output makes could -// contain it. -// -// One limit is deliberate: an output carries at most one unrealized claim, the widest, so a merge -// consuming fragments of two groups without completing either keeps the lineage of one. The other -// falls back to exact-prefix matching, which is where every split left it before claims existed. +// planMergeBlocks stamps the identity the outputs of a merge over src claim: see +// [bucketindex.PlanMerge]. func planMergeBlocks(src, out []*part) { - var ( - union bucketindex.Interval - level uint32 - claims []bucketindex.Claim - ) - - for _, p := range src { - level = max(level, p.level+1) - union = union.Union(p.blocks) - - if p.claim.Valid() { - claims = append(claims, p.claim) - } - } - - union, claims = realizeClaims(union, claims) - - if len(out) == 1 { - out[0].pending = &blockPlan{blocks: union, claim: widestClaim(claims), level: level} - - return - } - - group := &splitGroup{blocks: union, n: len(out)} - for _, p := range out { - p.pending = &blockPlan{group: group, level: level} - } -} - -// realizeClaims folds into held every claim whose group is wholly inside it, repeating while that -// keeps realizing more — a group whose members were themselves split resolves only after the inner -// one has. It returns what is covered and the claims still owed a member. -func realizeClaims(held bucketindex.Interval, claims []bucketindex.Claim) (bucketindex.Interval, []bucketindex.Claim) { - for len(claims) > 0 { - rest := claims[:0] - - for _, c := range claims { - if held.Contains(c.Group) { - held = held.Union(c.Blocks) - - continue - } - - rest = append(rest, c) - } - - if len(rest) == len(claims) { - break - } - - claims = rest + ents := make([]bucketindex.Entry, len(src)) + for i, p := range src { + ents[i] = bucketindex.Entry{Blocks: p.blocks, Claim: p.claim, Level: p.level} } - return held, claims -} - -func widestClaim(claims []bucketindex.Claim) bucketindex.Claim { - var best bucketindex.Claim - for _, c := range claims { - if c.Blocks.Len() > best.Blocks.Len() { - best = c - } + for i, plan := range bucketindex.PlanMerge(ents, len(out)) { + out[i].pending = plan } - - return best } // nextBlockLocked is [bucketindex.Index.NextBlock] over the whole state the commit under @@ -140,7 +43,7 @@ func widestClaim(claims []bucketindex.Claim) bucketindex.Claim { // writer's included, since its blocks are real claims and reusing one would make two different // parts share an identity — this engine's own parts, the holes it carries, and every want // outstanding or pending, whose part may yet be repaired back into the index. Caller holds e.mu. -func (e *Engine) nextBlockLocked(ix *bucketindex.Index) uint64 { +func (e *Engine) nextBlockLocked(ix *bucketindex.Index) bucketindex.Block { seed := bucketindex.Index{ Entries: slices.Concat(ix.Entries, e.holes), Wanted: slices.Concat(e.wants, e.pendingWants, e.adoptedWants, e.pendingHoles), @@ -151,51 +54,5 @@ func (e *Engine) nextBlockLocked(ix *bucketindex.Index) uint64 { seed.Entries = append(seed.Entries, bucketindex.Entry{Blocks: p.blocks, Claim: p.claim}) } - return seed.NextBlock() -} - -// groupRun is the run of blocks one commit attempt allocated to a split group, and how far into it -// the fragments assigned so far have got. -type groupRun struct { - run bucketindex.Interval - next uint64 -} - -// allocateBlocks turns a pending plan into a real identity, taking numbers from *next. -// -// A split group takes its whole run at once, before any of its fragments is numbered, so every -// fragment carries the same claim and can name the siblings a repair still has to fetch. -func allocateBlocks( - id *blockPlan, next *uint64, groups map[*splitGroup]*groupRun, -) (bucketindex.Interval, bucketindex.Claim) { - if id.blocks.Valid() { - return id.blocks, id.claim - } - - if id.group == nil { - iv := bucketindex.Interval{Min: *next, Max: *next} - *next++ - - return iv, id.claim - } - - g, ok := groups[id.group] - if !ok { - g = &groupRun{ - run: bucketindex.Interval{Min: *next, Max: *next + uint64(id.group.n) - 1}, - next: *next, - } - groups[id.group] = g - *next = g.run.Max + 1 - } - - iv := bucketindex.Interval{Min: g.next, Max: g.next} - g.next++ - - claim := bucketindex.Claim{Blocks: id.group.blocks, Group: g.run} - if !claim.Valid() { - return iv, bucketindex.Claim{} - } - - return iv, claim + return seed.NextBlock(e.term()) } diff --git a/engine/engine.go b/engine/engine.go index 14d1a5da..90951e4b 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -279,10 +279,10 @@ type Engine struct { // lostParts is the index's monotone data-loss counter, carried across commits and raised to a // rival's on rebase, so it is a cluster-visible fact and not a level a restart clears. lostParts uint64 - // allocated is the block-number high-water mark this engine last committed, carried across - // commits and raised to a rival's on rebase for the same reason: identity must stay unique over - // the shard's whole life, and the live set it could otherwise be derived from shrinks. - allocated uint64 + // allocated is the block high-water mark this engine last committed, carried across commits + // and raised to a rival's on rebase for the same reason: identity must stay unique over the + // shard's whole life, and the live set it could otherwise be derived from shrinks. + allocated bucketindex.Block // pendingHoles are the losses the next commit must acknowledge. Like pendingWants they are // held rather than applied on the spot, so a commit that never lands leaves the want // outstanding instead of half-discharged. diff --git a/engine/export_test.go b/engine/export_test.go index d47d9792..f9399301 100644 --- a/engine/export_test.go +++ b/engine/export_test.go @@ -101,7 +101,7 @@ func (e *Engine) LoadState() any { Indexed map[string]struct{} Holes []bucketindex.Entry LostParts uint64 - Allocated uint64 + Allocated bucketindex.Block IdentityDirty bool FlushedEpoch uint64 Epochs []bucketindex.WriterEpoch diff --git a/engine/index.go b/engine/index.go index 2ee965a9..c925e7cb 100644 --- a/engine/index.go +++ b/engine/index.go @@ -47,13 +47,16 @@ const indexCommitAttempts = 8 // part is dropped from the index that survives. It is a no-op for a head-only engine (no // backend), and refused while the engine is fenced ([Engine.fenceLocked]). Caller holds e.mu. func (e *Engine) updateIndexLocked(ctx context.Context) error { - return e.commitIndexLocked(ctx, nil) + return e.commitIndexLocked(ctx, e.term(), nil) } -// commitIndexLocked is [Engine.updateIndexLocked] that runs guard, when non-nil, after each rebase: -// an error from it abandons the commit, since the rival's parts it adopted may be ones the caller's -// own must not be committed beside. Caller holds e.mu. -func (e *Engine) commitIndexLocked(ctx context.Context, guard func() error) error { +// commitIndexLocked is [Engine.updateIndexLocked] for an operation that began under ownership term +// stamp. Every attempt first passes the tenure fence ([Engine.tenureLocked]), so a writer displaced +// while the operation ran — or one a rebase shows a later tenure has overtaken — commits nothing. +// guard, when non-nil, runs after each rebase: an error from it abandons the commit, since the +// rival's parts it adopted may be ones the caller's own must not be committed beside. Caller holds +// e.mu. +func (e *Engine) commitIndexLocked(ctx context.Context, stamp uint64, guard func() error) error { if e.cfg.Backend == nil { return nil } @@ -63,6 +66,10 @@ func (e *Engine) commitIndexLocked(ctx context.Context, guard func() error) erro } for range indexCommitAttempts { + if err := e.tenureLocked(stamp); err != nil { + return err + } + ix := e.nextIndexLocked(ctx) version, err := ix.Save(ctx, e.cfg.Backend, e.indexKey(), e.indexVersion) @@ -76,7 +83,8 @@ func (e *Engine) commitIndexLocked(ctx context.Context, guard func() error) erro // landed would hold a block the winner took, and the retry would not re-allocate. for i := range e.pendingBlocks { a := &e.pendingBlocks[i] - a.part.blocks, a.part.claim, a.part.level, a.part.pending = a.blocks, a.claim, a.level, nil + a.part.blocks, a.part.claim, a.part.level, a.part.term = a.blocks, a.claim, a.level, a.term + a.part.pending = nil } e.pendingBlocks = nil @@ -142,7 +150,7 @@ func (e *Engine) adoptIndexLocked(ctx context.Context) error { // not walk back: the counter only ever rises. e.holes = mergeHoles(e.holes, ix.Holes()) e.lostParts = max(e.lostParts, ix.LostParts) - e.allocated = max(e.allocated, ix.AllocatedBlocks) + e.allocated = bucketindex.MaxBlock(e.allocated, ix.AllocatedBlocks) e.foreign = foreignEntries(ix.Entries, e.indexed, e.removals, e.wants) e.openForeignLocked(ctx) @@ -255,32 +263,36 @@ func (e *Engine) nextIndexLocked(ctx context.Context) *bucketindex.Index { // Block numbers are allocated here, per attempt, because a rival's entries are only known // after a rebase: allocating once and reusing it across retries would hand a part a block the // winner already claimed. The assignments are held, not applied — see [Engine.updateIndexLocked]. - next := e.nextBlockLocked(ix) - groups := make(map[*splitGroup]*groupRun) + alloc := bucketindex.NewAllocator(e.nextBlockLocked(ix)) var assigned []blockAssignment + own := make([]bucketindex.Entry, 0, len(e.parts)) live := make(map[string]struct{}, len(e.parts)) + for _, p := range e.parts { - blocks, claim, level := p.blocks, p.claim, p.level - if id := p.pending; id != nil { - blocks, claim = allocateBlocks(id, &next, groups) - level = id.level + blocks, claim, level, term := p.blocks, p.claim, p.level, p.term + if plan := p.pending; plan != nil { + blocks, claim = alloc.Assign(plan) + level, term = plan.Level, alloc.Term() - assigned = append(assigned, blockAssignment{part: p, blocks: blocks, claim: claim, level: level}) + assigned = append(assigned, blockAssignment{part: p, blocks: blocks, claim: claim, level: level, term: term}) } - ix.Add(bucketindex.Entry{ + ent := bucketindex.Entry{ Prefix: p.prefix, MinTime: p.minTime, MaxTime: p.maxTime, - Blocks: blocks, Claim: claim, Level: level, - }) + Blocks: blocks, Claim: claim, Level: level, Term: term, + } + ix.Add(ent) + own = append(own, ent) live[p.prefix] = struct{}{} } - // One above the last block handed out, over every source this attempt considered — so the mark - // only ever rises, and a shard whose live set has since emptied never renumbers over a part it - // once held. - ix.AllocatedBlocks = next - 1 + e.dropSubsumedForeignLocked(ix, own) + + // The last block handed out, over every source this attempt considered — so the mark only ever + // rises, and a shard whose live set has since emptied never renumbers over a part it once held. + ix.AllocatedBlocks = alloc.Mark() e.pendingBlocks = assigned @@ -479,7 +491,7 @@ func (e *Engine) livePart( delete(open, ent.Prefix) p.minTime, p.maxTime = ent.MinTime, ent.MaxTime - p.blocks, p.claim, p.level = ent.Blocks, ent.Claim, ent.Level + p.blocks, p.claim, p.level, p.term = ent.Blocks, ent.Claim, ent.Level, ent.Term return p, nil } @@ -488,7 +500,7 @@ func (e *Engine) livePart( // still pending, so reusing it needs no write. func (p *part) matchesEntry(ent *bucketindex.Entry) bool { return p.pending == nil && p.minTime == ent.MinTime && p.maxTime == ent.MaxTime && - p.level == ent.Level && p.blocks.Equal(ent.Blocks) && p.claim.Equal(ent.Claim) + p.level == ent.Level && p.term == ent.Term && p.blocks.Equal(ent.Blocks) && p.claim.Equal(ent.Claim) } // indexLoad is the state a load replaces, read and opened into locals so that a load failing @@ -743,8 +755,9 @@ func (e *Engine) loadPartsLocked(ctx context.Context, mode loadMode) error { // One commit drops the gone parts from Entries and states the wants that replace them. It runs // before the rest of the load so the obligation is durable even if identity recovery fails. A // carried part's identity would be stamped by this commit, which does not hold flushMu, so the - // wants then stay pending for the next flush or merge to commit. - if len(l.lost) > 0 && mode == loadOwner && !l.carried { + // wants then stay pending for the next flush or merge to commit. So do they on a clustered engine + // without the shard's claim, which may not commit at all: the load is then an unclaimed one. + if len(l.lost) > 0 && mode == loadOwner && !l.carried && e.tenureLocked(e.term()) == nil { if err := e.updateIndexLocked(ctx); err != nil { return errors.Wrap(err, "record repair wants") } @@ -875,3 +888,41 @@ func colsTimeRange(cols *flushColumns) (minTime, maxTime int64) { return minTime, maxTime } + +// tenureLocked is the commit fence of a clustered writer ([bucketindex.CheckTenure]) for an +// operation that began under term stamp, checked against the term of the index the commit builds on. +// A writer with no cluster has no tenure to lose. Caller holds e.mu. +func (e *Engine) tenureLocked(stamp uint64) error { + if e.cfg.Term == nil { + return nil + } + + return bucketindex.CheckTenure(stamp, e.term(), e.generation.Term) +} + +// dropSubsumedForeignLocked takes out of ix, and tombstones, the adopted entries whose every row this +// engine's own entries hold — the same merge a displaced tenure committed before this one, resolved +// to the later tenure, or a part a merge here has since consumed. Carrying one forward would keep a +// set of rows live twice. Caller holds e.mu. +func (e *Engine) dropSubsumedForeignLocked(ix *bucketindex.Index, own []bucketindex.Entry) { + if len(e.foreign) == 0 { + return + } + + gone := bucketindex.Subsumed(e.foreign, own) + if len(gone) == 0 { + return + } + + for prefix := range gone { + ix.Remove(prefix) + delete(e.foreignParts, prefix) + e.removals = append(e.removals, bucketindex.Removal{Prefix: prefix, Generation: e.generation}) + } + + e.foreign = slices.DeleteFunc(e.foreign, func(ent bucketindex.Entry) bool { + _, ok := gone[ent.Prefix] + + return ok + }) +} diff --git a/engine/merge.go b/engine/merge.go index 8d657d82..f6b12b8f 100644 --- a/engine/merge.go +++ b/engine/merge.go @@ -100,6 +100,9 @@ func (e *Engine) merge(ctx context.Context, opts MergeOptions) (mergeResult, err e.flushMu.Lock() defer e.flushMu.Unlock() + // The tenure the inputs were chosen under: a merge that outlives it commits nothing. + stamp := e.term() + // Plan (under lock): snapshot the source parts (immutable backing). Output part ids are minted one // at a time, as the parts are written. e.mu.Lock() @@ -246,7 +249,7 @@ func (e *Engine) merge(ctx context.Context, opts MergeOptions) (mergeResult, err committed := e.parts e.parts = replaceParts(e.parts, removed, newParts...) - if err = e.commitIndexLocked(ctx, rollupGuard(readable, plan.marker, e)); err != nil { + if err = e.commitIndexLocked(ctx, stamp, rollupGuard(readable, plan.marker, e)); err != nil { e.parts = committed if !errors.Is(err, errRollupConflict) { diff --git a/engine/part.go b/engine/part.go index 164628d1..f74b81d4 100644 --- a/engine/part.go +++ b/engine/part.go @@ -340,11 +340,13 @@ type part struct { // claim is the ancestry this part holds only jointly with the rest of its split group; unset // for every part a merge wrote whole — see [bucketindex.Claim]. claim bucketindex.Claim + // term is the ownership term of the tenure that wrote the part ([bucketindex.Entry.Term]). + term uint64 // pending is the identity a part this engine just wrote is waiting to be assigned; nil for one // opened from an index, whose identity is whatever that index recorded — including the unset // one of a part written before format v5. See blockid.go. - pending *blockPlan + pending *bucketindex.Plan // refs counts in-flight fetches reading this part lock-free. A fetch acquires (under the engine // lock, while the part is still live) the parts it will read and releases them when done; a retired diff --git a/engine/repair.go b/engine/repair.go index c09a7880..5ef8ac48 100644 --- a/engine/repair.go +++ b/engine/repair.go @@ -64,7 +64,7 @@ func (h repairHost) Reclaim(ctx context.Context) { h.e.reclaimRet func (h repairHost) Identity(p *part) bucketindex.Entry { return bucketindex.Entry{ Prefix: p.prefix, MinTime: p.minTime, MaxTime: p.maxTime, - Blocks: p.blocks, Claim: p.claim, Level: p.level, + Blocks: p.blocks, Claim: p.claim, Level: p.level, Term: p.term, } } @@ -74,7 +74,7 @@ func (h repairHost) Open(ctx context.Context, prefix string) (*part, error) { func (h repairHost) AdoptLocked(ctx context.Context, p *part, ent *bucketindex.Entry) error { p.minTime, p.maxTime = ent.MinTime, ent.MaxTime - p.blocks, p.claim, p.level = ent.Blocks, ent.Claim, ent.Level + p.blocks, p.claim, p.level, p.term = ent.Blocks, ent.Claim, ent.Level, ent.Term _, err := h.e.registerPartIdentitiesLocked(ctx, ent.Prefix) diff --git a/engine/splitlineage_test.go b/engine/splitlineage_test.go index 92165e32..88424b94 100644 --- a/engine/splitlineage_test.go +++ b/engine/splitlineage_test.go @@ -111,7 +111,7 @@ func flushInputs(ctx context.Context, t *testing.T, be backend.Backend, split in for i, id := range ids { ent := byPrefix[lostPrefix+"/"+id] - require.Equal(t, bucketindex.Interval{Min: uint64(i + 1), Max: uint64(i + 1)}, ent.Blocks) + require.Equal(t, bucketindex.Range(0, uint64(i+1), uint64(i+1)), ent.Blocks) out = append(out, ent) } @@ -159,7 +159,7 @@ func TestSplitMergeAllocatesAboveItsInputs(t *testing.T) { _, fragments := splitMerge(ctx, t, be, inputs) for _, f := range fragments { - assert.Greater(t, f.Blocks.Min, inputs[2].Blocks.Max, "fragment %+v is numbered below a retired input", f) + assert.Positive(t, f.Blocks.Min.Compare(inputs[2].Blocks.Max), "fragment %+v is numbered below a retired input", f) for _, in := range inputs { assert.False(t, f.Supersedes(in), "fragment %+v holds a fraction of %+v and must not claim it", f, in) diff --git a/internal/enginetest/blockid.go b/internal/enginetest/blockid.go index 98f94bf3..20d2f2d5 100644 --- a/internal/enginetest/blockid.go +++ b/internal/enginetest/blockid.go @@ -54,7 +54,7 @@ func (k Kind) stripBlocks(ctx context.Context, t *testing.T, be backend.Backend, // An index that predates block identity predates the allocation high-water mark too, so the // simulation has to drop it as well or the migrated part numbers above a mark v5 never wrote. - ix.AllocatedBlocks = 0 + ix.AllocatedBlocks = bucketindex.Block{} _, err = ix.Save(ctx, be, k.indexKey(), version) require.NoError(t, err) @@ -71,7 +71,7 @@ func flushAllocatesABlock(t *testing.T, k Kind) { got := blocksByPrefix(k.loadIndex(t, be)) for i, id := range ids { ent := got[k.Prefix+"/"+id] - assert.Equal(t, bucketindex.Interval{Min: uint64(i + 1), Max: uint64(i + 1)}, ent.Blocks) + assert.Equal(t, bucketindex.Range(0, uint64(i+1), uint64(i+1)), ent.Blocks) assert.Zero(t, ent.Level) } } @@ -93,7 +93,7 @@ func mergeUnionsItsInputs(t *testing.T, k Kind) { require.Len(t, ix.Entries, 1, "the forced merge collapses the part set") out := ix.Entries[0] - assert.Equal(t, bucketindex.Interval{Min: 1, Max: 3}, out.Blocks) + assert.Equal(t, bucketindex.Range(0, 1, 3), out.Blocks) assert.Equal(t, uint32(1), out.Level) for _, id := range ids { @@ -126,7 +126,7 @@ func mergedNeighboursDoNotDischargeAWant(t *testing.T, k Kind) { lost := ix.Wanted[0] require.True(t, lost.Blocks.Valid(), "the want carries the identity the flush allocated") - require.Equal(t, bucketindex.Interval{Min: 2, Max: 2}, lost.Blocks) + require.Equal(t, bucketindex.Range(0, 2, 2), lost.Blocks) require.NoError(t, r.ForceMerge(ctx)) @@ -167,7 +167,7 @@ func blocksSurviveRestart(t *testing.T, k Kind) { fresh := k.partDirs(ctx, t, be) require.Len(t, fresh, 3) - assert.Equal(t, bucketindex.Interval{Min: 3, Max: 3}, after[k.Prefix+"/"+fresh[2]].Blocks, + assert.Equal(t, bucketindex.Range(0, 3, 3), after[k.Prefix+"/"+fresh[2]].Blocks, "the new owner allocates above what it inherited") } @@ -203,7 +203,7 @@ func rebaseAllocatesAboveTheRival(t *testing.T, k Kind) { other := &bucketindex.Index{Generation: bucketindex.Generation{Term: 1, Counter: 1}} other.Add(bucketindex.Entry{ Prefix: rival, MinTime: 1, MaxTime: 2, - Blocks: bucketindex.Interval{Min: 1, Max: 1}, + Blocks: bucketindex.Range(0, 1, 1), }) _, err := other.Save(ctx, inner, k.indexKey(), backend.VersionAbsent) require.NoError(t, err) @@ -216,11 +216,11 @@ func rebaseAllocatesAboveTheRival(t *testing.T, k Kind) { require.Len(t, ix.Entries, 2) got := blocksByPrefix(ix) - require.Equal(t, bucketindex.Interval{Min: 1, Max: 1}, got[rival].Blocks, "the winner keeps the block it claimed") + require.Equal(t, bucketindex.Range(0, 1, 1), got[rival].Blocks, "the winner keeps the block it claimed") mine := k.partDirs(ctx, t, inner) require.Len(t, mine, 1) - assert.Equal(t, bucketindex.Interval{Min: 2, Max: 2}, got[k.Prefix+"/"+mine[0]].Blocks, "the loser re-allocates above the winner") + assert.Equal(t, bucketindex.Range(0, 2, 2), got[k.Prefix+"/"+mine[0]].Blocks, "the loser re-allocates above the winner") for a := range got { for b := range got { @@ -253,7 +253,7 @@ func outstandingWantHoldsItsBlock(t *testing.T, k Kind) { require.Len(t, ix.Wanted, 1) fresh := k.partDirs(ctx, t, be) - assert.Equal(t, bucketindex.Interval{Min: 3, Max: 3}, blocksByPrefix(ix)[k.Prefix+"/"+fresh[len(fresh)-1]].Blocks, + assert.Equal(t, bucketindex.Range(0, 3, 3), blocksByPrefix(ix)[k.Prefix+"/"+fresh[len(fresh)-1]].Blocks, "the wanted part's block 2 is not handed out again") } @@ -274,7 +274,7 @@ func preV5PartsMigrateOnMerge(t *testing.T, k Kind) { ix := k.loadIndex(t, be) require.Len(t, ix.Entries, 1) - assert.Equal(t, bucketindex.Interval{Min: 1, Max: 1}, ix.Entries[0].Blocks, "the migrated part takes the first free block") + assert.Equal(t, bucketindex.Range(0, 1, 1), ix.Entries[0].Blocks, "the migrated part takes the first free block") assert.Equal(t, uint32(1), ix.Entries[0].Level) } @@ -296,7 +296,7 @@ func mixedMergeInheritsTheKnownInputs(t *testing.T, k Kind) { ix := k.loadIndex(t, be) require.Len(t, ix.Entries, 1) - assert.Equal(t, bucketindex.Interval{Min: 2, Max: 3}, ix.Entries[0].Blocks) + assert.Equal(t, bucketindex.Range(0, 2, 3), ix.Entries[0].Blocks) assert.Equal(t, uint32(1), ix.Entries[0].Level) } @@ -321,12 +321,12 @@ func blockNumbersSurviveAnEmptiedShard(t *testing.T, k Kind) { ix := k.loadIndex(t, be) require.Empty(t, ix.Entries) - require.EqualValues(t, 3, ix.NextBlock(), "the high-water mark outlives every part it numbered") + require.Equal(t, bucketindex.Block{N: 3}, ix.NextBlock(0), "the high-water mark outlives every part it numbered") e.Append(t, api(1<<41, 1)) require.NoError(t, e.Flush(ctx)) ix = k.loadIndex(t, be) require.Len(t, ix.Entries, 1) - require.Equal(t, bucketindex.Interval{Min: 3, Max: 3}, ix.Entries[0].Blocks, "block numbers are never reused within a shard") + require.Equal(t, bucketindex.Range(0, 3, 3), ix.Entries[0].Blocks, "block numbers are never reused within a shard") } diff --git a/internal/enginetest/enginetest.go b/internal/enginetest/enginetest.go index fcf5654e..8d7a5cc3 100644 --- a/internal/enginetest/enginetest.go +++ b/internal/enginetest/enginetest.go @@ -291,6 +291,10 @@ var suite = []struct { {"SplitBrainMergeSameInputs", splitBrainMergeSameInputs}, {"SplitBrainMergeOverlappingInputs", splitBrainMergeOverlappingInputs}, {"DisplacedWriterKeepsItsTerm", displacedWriterKeepsItsTerm}, + {"SplitBrainMergeResolvesToTheLaterTenure", splitBrainMergeResolvesToTheLaterTenure}, + {"TenuresAllocateDisjointBlocks", tenuresAllocateDisjointBlocks}, + {"LapsedClaimCommitsNothing", lapsedClaimCommitsNothing}, + {"WritingTermSurvivesTheNextTenure", writingTermSurvivesTheNextTenure}, } // Run runs every suite test against k, each as /. diff --git a/internal/enginetest/heldpart.go b/internal/enginetest/heldpart.go index 551b0691..61e3ad0b 100644 --- a/internal/enginetest/heldpart.go +++ b/internal/enginetest/heldpart.go @@ -29,7 +29,7 @@ func repairDischargedByPartHeldOnDisk(t *testing.T, k Kind) { held := flushTwo(t, e)[0] // The index says the part is gone; the objects never left. - e.LosePart(held, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(held, bucketindex.Range(0, 1, 1)) require.Equal(t, []string{held}, e.WantPrefixes()) require.NoError(t, e.Merge(ctx, 0)) diff --git a/internal/enginetest/hole.go b/internal/enginetest/hole.go index b77c21ec..04b9f4d5 100644 --- a/internal/enginetest/hole.go +++ b/internal/enginetest/hole.go @@ -148,7 +148,7 @@ func holeRevokedByExactPrefix(t *testing.T, k Kind) { CopyObjects(t, be, peer, lost) dropObjects(t, be, lost) - e.LosePart(lost, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(lost, bucketindex.Range(0, 1, 1)) mergeTimes(t, e, 3) require.Len(t, e.Holes(), 1) @@ -182,14 +182,14 @@ func holeRevokedByLostSuccessor(t *testing.T, k Kind) { parts := flushTwo(t, e) lost, successor := parts[0], parts[1] - e.SetPartBlocks(successor, bucketindex.Interval{Min: 1, Max: 4}, 2) + e.SetPartBlocks(successor, bucketindex.Range(0, 1, 4), 2) CopyObjects(t, be, peer, successor) dropObjects(t, be, lost) - e.LosePart(lost, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(lost, bucketindex.Range(0, 1, 1)) // The successor is local, so it would discharge the want before a hole could form: take it out // of the way and let the loss be acknowledged first. - e.LosePart(successor, bucketindex.Interval{Min: 1, Max: 4}) + e.LosePart(successor, bucketindex.Range(0, 1, 4)) dropObjects(t, be, successor) mergeTimes(t, e, 3) @@ -200,7 +200,7 @@ func holeRevokedByLostSuccessor(t *testing.T, k Kind) { return bucketindex.Entry{ Prefix: successor, MinTime: 200, MaxTime: 200, - Blocks: bucketindex.Interval{Min: 1, Max: 4}, Level: 2, + Blocks: bucketindex.Range(0, 1, 4), Level: 2, }, bucketindex.WantSatisfied, nil }) @@ -223,15 +223,15 @@ func holeRevokedByContainingSuccessor(t *testing.T, k Kind) { parts := flushTwo(t, e) for i, p := range parts { - e.SetPartBlocks(p, bucketindex.Interval{Min: uint64(i + 1), Max: uint64(i + 1)}, 0) + e.SetPartBlocks(p, bucketindex.Range(0, uint64(i+1), uint64(i+1)), 0) } for _, p := range parts { dropObjects(t, be, p) } - e.LosePart(parts[0], bucketindex.Interval{Min: 1, Max: 1}) - e.LosePart(parts[1], bucketindex.Interval{Min: 2, Max: 2}) + e.LosePart(parts[0], bucketindex.Range(0, 1, 1)) + e.LosePart(parts[1], bucketindex.Range(0, 2, 2)) // The peer merged both blocks away into one level-1 part. successor := k.mergedPeerPart(t, peerBE) @@ -244,7 +244,7 @@ func holeRevokedByContainingSuccessor(t *testing.T, k Kind) { return bucketindex.Entry{ Prefix: successor, MinTime: 100, MaxTime: 200, - Blocks: bucketindex.Interval{Min: 1, Max: 2}, Level: 1, + Blocks: bucketindex.Range(0, 1, 2), Level: 1, }, bucketindex.WantSatisfied, nil }) @@ -298,7 +298,7 @@ func holeNotOfferedToAPeer(t *testing.T, k Kind) { lost := loseFirstOfTwo(t, e, be) mergeTimes(t, e, 3) - _, ok := k.loadIndex(t, be).Satisfying(bucketindex.Want{Prefix: lost, Blocks: bucketindex.Interval{Min: 1, Max: 1}}) + _, ok := k.loadIndex(t, be).Satisfying(bucketindex.Want{Prefix: lost, Blocks: bucketindex.Range(0, 1, 1)}) assert.False(t, ok) } diff --git a/internal/enginetest/repair.go b/internal/enginetest/repair.go index 4e24b6cd..54a49651 100644 --- a/internal/enginetest/repair.go +++ b/internal/enginetest/repair.go @@ -75,7 +75,7 @@ func loseFirstOfTwo(t *testing.T, e Engine, be backend.Backend) string { lost := flushTwo(t, e)[0] dropObjects(t, be, lost) - e.LosePart(lost, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(lost, bucketindex.Range(0, 1, 1)) e.Append(t, api(300, 3)) require.NoError(t, e.Flush(context.Background())) diff --git a/internal/enginetest/repairfetch.go b/internal/enginetest/repairfetch.go index b6655c87..87db9613 100644 --- a/internal/enginetest/repairfetch.go +++ b/internal/enginetest/repairfetch.go @@ -39,7 +39,7 @@ func loseOneOfThree(t *testing.T, e Engine, be backend.Backend) string { lost := flushThree(t, e)[0] dropObjects(t, be, lost) - e.LosePart(lost, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(lost, bucketindex.Range(0, 1, 1)) require.True(t, slices.Contains(e.WantPrefixes(), lost)) @@ -79,7 +79,7 @@ func repairFetchesWantedPartFromPeer(t *testing.T, k Kind) { CopyObjects(t, be, peer, lost) dropObjects(t, be, lost) - e.LosePart(lost, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(lost, bucketindex.Range(0, 1, 1)) f.SetAnswer(satisfyFrom(t, peer, be)) @@ -110,11 +110,11 @@ func repairDischargedByContainingSuccessor(t *testing.T, k Kind) { parts := flushTwo(t, e) lost, kept := parts[0], parts[1] - e.SetPartBlocks(lost, bucketindex.Interval{Min: 1, Max: 1}, 0) - e.SetPartBlocks(kept, bucketindex.Interval{Min: 2, Max: 2}, 0) + e.SetPartBlocks(lost, bucketindex.Range(0, 1, 1), 0) + e.SetPartBlocks(kept, bucketindex.Range(0, 2, 2), 0) dropObjects(t, be, lost) - e.LosePart(lost, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(lost, bucketindex.Range(0, 1, 1)) // The peer merged both blocks away into one level-1 part; the wanted prefix is gone there too. successor := k.mergedPeerPart(t, peerBE) @@ -124,7 +124,7 @@ func repairDischargedByContainingSuccessor(t *testing.T, k Kind) { return bucketindex.Entry{ Prefix: successor, MinTime: 100, MaxTime: 200, - Blocks: bucketindex.Interval{Min: 1, Max: 2}, Level: 1, + Blocks: bucketindex.Range(0, 1, 2), Level: 1, }, bucketindex.WantSatisfied, nil }) @@ -158,10 +158,10 @@ func repairDischargedByLocalPart(t *testing.T, k Kind) { parts := flushTwo(t, e) // The surviving part already covers the wanted blocks at a higher level. - e.SetPartBlocks(parts[1], bucketindex.Interval{Min: 1, Max: 4}, 1) + e.SetPartBlocks(parts[1], bucketindex.Range(0, 1, 4), 1) dropObjects(t, be, parts[0]) - e.LosePart(parts[0], bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(parts[0], bucketindex.Range(0, 1, 1)) require.NoError(t, e.Merge(ctx, 0)) @@ -282,7 +282,7 @@ func repairCommitFailureKeepsWant(t *testing.T, k Kind) { lost := flushThree(t, e)[0] CopyObjects(t, be, peer, lost) dropObjects(t, be, lost) - e.LosePart(lost, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(lost, bucketindex.Range(0, 1, 1)) f.SetAnswer(satisfyFrom(t, peer, be)) @@ -308,7 +308,7 @@ func repairAsksTheFetcherOncePerCycle(t *testing.T, k Kind) { parts := flushThree(t, e) for i, p := range parts { dropObjects(t, be, p) - e.LosePart(p, bucketindex.Interval{Min: uint64(i + 1), Max: uint64(i + 1)}) + e.LosePart(p, bucketindex.Range(0, uint64(i+1), uint64(i+1))) } require.Len(t, e.WantPrefixes(), len(parts)) @@ -334,16 +334,16 @@ func repairCoveredWantIsNotAFailure(t *testing.T, k Kind) { parts := flushThree(t, e) first, second, kept := parts[0], parts[1], parts[2] - e.SetPartBlocks(first, bucketindex.Interval{Min: 1, Max: 1}, 0) - e.SetPartBlocks(second, bucketindex.Interval{Min: 2, Max: 2}, 0) - e.SetPartBlocks(kept, bucketindex.Interval{Min: 3, Max: 3}, 0) + e.SetPartBlocks(first, bucketindex.Range(0, 1, 1), 0) + e.SetPartBlocks(second, bucketindex.Range(0, 2, 2), 0) + e.SetPartBlocks(kept, bucketindex.Range(0, 3, 3), 0) for _, lost := range []string{first, second} { dropObjects(t, be, lost) } - e.LosePart(first, bucketindex.Interval{Min: 1, Max: 1}) - e.LosePart(second, bucketindex.Interval{Min: 2, Max: 2}) + e.LosePart(first, bucketindex.Range(0, 1, 1)) + e.LosePart(second, bucketindex.Range(0, 2, 2)) successor := k.mergedPeerPart(t, peerBE) phantom := k.phantom() @@ -357,13 +357,13 @@ func repairCoveredWantIsNotAFailure(t *testing.T, k Kind) { return bucketindex.Entry{ Prefix: successor, MinTime: 100, MaxTime: 200, - Blocks: bucketindex.Interval{Min: 1, Max: 2}, Level: 1, + Blocks: bucketindex.Range(0, 1, 2), Level: 1, }, bucketindex.WantSatisfied, nil } return bucketindex.Entry{ Prefix: phantom, MinTime: 200, MaxTime: 200, - Blocks: bucketindex.Interval{Min: 2, Max: 2}, + Blocks: bucketindex.Range(0, 2, 2), }, bucketindex.WantSatisfied, nil }) diff --git a/internal/enginetest/splitlineage.go b/internal/enginetest/splitlineage.go index 77ff8206..0d2ea639 100644 --- a/internal/enginetest/splitlineage.go +++ b/internal/enginetest/splitlineage.go @@ -94,7 +94,7 @@ func splitOfSeveralInputsKeepsEveryLineage(t *testing.T, k Kind) { f := &fragments[i] for j := range inputs { in := &inputs[j] - assert.Greater(t, f.Blocks.Min, in.Blocks.Max, "fragment %+v is numbered below retired %+v", f, in) + assert.Positive(t, f.Blocks.Min.Compare(in.Blocks.Max), "fragment %+v is numbered below retired %+v", f, in) assert.False(t, f.Supersedes(*in), "fragment %+v holds a fraction of %+v and must not claim it", f, in) } } diff --git a/internal/enginetest/tenure.go b/internal/enginetest/tenure.go index cddc85ba..e453cce8 100644 --- a/internal/enginetest/tenure.go +++ b/internal/enginetest/tenure.go @@ -13,7 +13,6 @@ import ( "github.com/oteldb/storage/backend" "github.com/oteldb/storage/backend/bucketindex" "github.com/oteldb/storage/backend/faultbackend" - "github.com/oteldb/storage/internal/reproduce" ) // The tests below are two tenures of one shard over a shared object store: writer A under term 1, @@ -99,11 +98,10 @@ func (k Kind) requireStoredOnce(t *testing.T, be backend.Backend, want []Row) { } // splitBrainMergeSameInputs: A's merge of {P1,P2} outlives its lease, B takes the shard and merges -// the same two parts, then A's merge commits. Both outputs cover blocks 1-2 at level 1. +// the same two parts, then A's merge tries to commit. Both outputs cover blocks 1-2 at level 1; the +// rebase shows A a later tenure's index, so A's commit is refused and B's output is the one live. func splitBrainMergeSameInputs(t *testing.T, k Kind) { t.Helper() - reproduce.Unfixed(t, 725, "a rebased merge adopts the rival's output as foreign and commits its own "+ - "beside it, so both outputs of the same inputs stay live under one block identity") ctx := context.Background() inner := backend.Memory() @@ -119,17 +117,16 @@ func splitBrainMergeSameInputs(t *testing.T, k Kind) { require.NoError(t, b.ForceMerge(ctx)) require.Equal(t, 1, b.PartCount(), "the new owner merged both parts") - require.NoError(t, commitA()) + require.ErrorIs(t, commitA(), bucketindex.ErrSuperseded, "the displaced merge commits nothing") k.requireStoredOnce(t, inner, want) } // splitBrainMergeOverlappingInputs: A merges {P1,P2} past its lease while B, which sees P1 as sealed -// under its smaller merge cap, flushes P3 and merges {P2,P3}. P2's rows are in both outputs. +// under its smaller merge cap, flushes P3 and merges {P2,P3}. P2's rows would be in both outputs, so +// A's commit is refused. func splitBrainMergeOverlappingInputs(t *testing.T, k Kind) { t.Helper() - reproduce.Unfixed(t, 725, "a rebased merge adopts the rival's output as foreign and commits its own "+ - "beside it, so the input both merged is live twice") ctx := context.Background() inner := backend.Memory() @@ -164,7 +161,7 @@ func splitBrainMergeOverlappingInputs(t *testing.T, k Kind) { require.ElementsMatch(t, []Part{parts[1], p3[0]}, removedParts(before, b.Parts()), "the new owner merges P2 and P3, leaving P1 sealed") - require.NoError(t, commitA()) + require.ErrorIs(t, commitA(), bucketindex.ErrSuperseded, "the displaced merge commits nothing") want := append(slices.Clone(big), api(2000, 2000), api(3000, 3000)) k.requireStoredOnce(t, inner, want) @@ -174,8 +171,6 @@ func splitBrainMergeOverlappingInputs(t *testing.T, k Kind) { // hand A the term B's index carries, or A's commit supersedes the owner's tenure with its own write. func displacedWriterKeepsItsTerm(t *testing.T, k Kind) { t.Helper() - reproduce.Unfixed(t, 725, "a rebase adopts the rival's higher generation and Generation.Next keeps its term, "+ - "so the displaced writer commits under the new owner's term") ctx := context.Background() be := backend.Memory() @@ -192,7 +187,7 @@ func displacedWriterKeepsItsTerm(t *testing.T, k Kind) { require.Equal(t, uint64(ownerTerm), owner.Term) a.Append(t, api(300, 3)) - _ = a.Flush(ctx) + require.ErrorIs(t, a.Flush(ctx), bucketindex.ErrSuperseded) got := k.loadIndex(t, be).Generation t.Logf("owner committed %+v, index after the displaced flush %+v", owner, got) diff --git a/internal/enginetest/tenurefence.go b/internal/enginetest/tenurefence.go new file mode 100644 index 00000000..5b62c021 --- /dev/null +++ b/internal/enginetest/tenurefence.go @@ -0,0 +1,161 @@ +package enginetest + +import ( + "context" + "strings" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/oteldb/storage/backend" + "github.com/oteldb/storage/backend/bucketindex" + "github.com/oteldb/storage/backend/faultbackend" +) + +// splitBrainMergeResolvesToTheLaterTenure is the other order of splitBrainMergeSameInputs: the +// displaced tenure A still holds its claim when it commits its merge, and B's merge of the same two +// parts lands second. B's rebase adopts A's output, finds its own output to be the same identity +// written by a later tenure, and retires A's rather than keeping both. +func splitBrainMergeResolvesToTheLaterTenure(t *testing.T, k Kind) { + t.Helper() + + ctx := context.Background() + inner := backend.Memory() + bBe := faultbackend.Wrap(inner) + want := []Row{api(100, 1), api(200, 2)} + + a := k.openTenure(t, inner, displacedTerm, 0) + k.flushEach(t, a, inner, want...) + + b := k.openTenure(t, bBe, ownerTerm, 0) + commitB := stallMerge(t, b, gateIndexCommit(bBe)) + + require.NoError(t, a.ForceMerge(ctx)) + require.NoError(t, commitB()) + + k.requireStoredOnce(t, inner, want) + + ix := k.loadIndex(t, inner) + require.Len(t, ix.Entries, 1) + assert.EqualValues(t, ownerTerm, ix.Entries[0].Term, "the later tenure's output is the one kept") + assert.Len(t, ix.Removed, 3, "both inputs and the earlier output are tombstoned, not dropped silently") +} + +// tenuresAllocateDisjointBlocks is #725's block collision: two tenures whose indexes diverged — a +// displaced owner and its successor, each on its own copy — both flush the next part. Allocation is +// scoped to the tenure's term, so the two parts never share an identity, and a later merge of both +// supersedes each of them. +func tenuresAllocateDisjointBlocks(t *testing.T, k Kind) { + t.Helper() + + ctx := context.Background() + aBe, bBe := backend.Memory(), backend.Memory() + + a := k.openTenure(t, aBe, displacedTerm, 0) + k.flushEach(t, a, aBe, api(100, 1)) + + copyObjects(ctx, t, aBe, bBe, k.Prefix) + b := k.openTenure(t, bBe, ownerTerm, 0) + + a.Append(t, api(200, 2)) + require.NoError(t, a.Flush(ctx)) + b.Append(t, api(300, 3)) + require.NoError(t, b.Flush(ctx)) + + fresh := func(be backend.Backend) bucketindex.Entry { + t.Helper() + + ix := k.loadIndex(t, be) + require.Len(t, ix.Entries, 2) + + return ix.Entries[1] + } + + pa, pb := fresh(aBe), fresh(bBe) + t.Logf("displaced %v, successor %v", pa.Blocks, pb.Blocks) + assert.Equal(t, bucketindex.TermBlocks(displacedTerm, 2), pa.Blocks) + assert.Equal(t, bucketindex.TermBlocks(ownerTerm, 1), pb.Blocks, "a new tenure numbers under its own term") + assert.False(t, bucketindex.LineageOf(nil).Overlaps(pa, pb), "the two parts share no identity") + + require.NoError(t, b.ForceMerge(ctx)) + merged := k.loadIndex(t, bBe).Entries + require.Len(t, merged, 1) + assert.True(t, merged[0].Blocks.Contains(pb.Blocks)) + assert.False(t, merged[0].Blocks.Contains(pa.Blocks), "the successor never merged the displaced part") +} + +// writingTermSurvivesTheNextTenure pins that a part's writing term is part of its identity, not of +// the engine that holds it: a later tenure loading the index and committing its own part carries +// every earlier entry's term through unchanged. +func writingTermSurvivesTheNextTenure(t *testing.T, k Kind) { + t.Helper() + + ctx := context.Background() + be := backend.Memory() + + a := k.openTenure(t, be, displacedTerm, 0) + k.flushEach(t, a, be, api(100, 1)) + + b := k.openTenure(t, be, ownerTerm, 0) + b.Append(t, api(200, 2)) + require.NoError(t, b.Flush(ctx)) + + ix := k.loadIndex(t, be) + require.Len(t, ix.Entries, 2) + + terms := map[uint64]bucketindex.Interval{} + for i := range ix.Entries { + terms[ix.Entries[i].Term] = ix.Entries[i].Blocks + } + + assert.Equal(t, map[uint64]bucketindex.Interval{ + displacedTerm: bucketindex.TermBlocks(displacedTerm, 1), + ownerTerm: bucketindex.TermBlocks(ownerTerm, 1), + }, terms) + assert.Equal(t, bucketindex.Block{Term: ownerTerm, N: 1}, ix.AllocatedBlocks) +} + +// lapsedClaimCommitsNothing is the commit fence over a writer's own term: a flush while the claim is +// gone is refused and keeps its rows, and a merge whose tenure ended and restarted while it ran is +// refused too, since it chose its inputs under a tenure that is over. +func lapsedClaimCommitsNothing(t *testing.T, k Kind) { + t.Helper() + + ctx := context.Background() + inner := backend.Memory() + be := faultbackend.Wrap(inner) + + var term atomic.Uint64 + + term.Store(1) + + e := k.Open(t, Config{Backend: be, Term: term.Load}) + require.NoError(t, e.LoadParts(ctx)) + k.flushEach(t, e, inner, api(100, 1), api(200, 2)) + + term.Store(0) + e.Append(t, api(300, 3)) + require.ErrorIs(t, e.Flush(ctx), bucketindex.ErrSuperseded, "no claim, no commit") + assert.Len(t, k.loadIndex(t, inner).Entries, 2) + + term.Store(3) + e.Append(t, api(400, 4)) + require.NoError(t, e.Flush(ctx), "the carried part commits with the next flush once the claim is back") + assert.EqualValues(t, 3, k.loadIndex(t, inner).Generation.Term) + assert.Len(t, k.loadIndex(t, inner).Entries, 4) + + // Held while it writes its output, so the tenure ends and restarts before it reaches the commit. + gate := faultbackend.NewGate() + be.Add(gate.Rule(faultbackend.Write, func(op faultbackend.Op) bool { + return !strings.HasSuffix(op.Key, "/"+bucketindex.Object) + })) + + commit := stallMerge(t, e, gate) + term.Store(4) + require.ErrorIs(t, commit(), bucketindex.ErrSuperseded, "a merge that outlived its tenure commits nothing") + + assert.Len(t, k.loadIndex(t, inner).Entries, 4) + assert.Equal(t, []Row{api(100, 1), api(200, 2), api(300, 3), api(400, 4)}, sortedRows(t, e)) +} diff --git a/internal/enginetest/wants.go b/internal/enginetest/wants.go index cda89235..d36d22c3 100644 --- a/internal/enginetest/wants.go +++ b/internal/enginetest/wants.go @@ -120,7 +120,7 @@ func unattemptedWantKeepsAbsenceEvidence(t *testing.T, k Kind) { require.Less(t, b, lost) blockers[b] = struct{}{} - e.LosePart(b, bucketindex.Interval{Min: uint64(100 + i), Max: uint64(100 + i)}) + e.LosePart(b, bucketindex.Range(0, uint64(100+i), uint64(100+i))) } before := len(f.Asks()) @@ -131,7 +131,7 @@ func unattemptedWantKeepsAbsenceEvidence(t *testing.T, k Kind) { require.NotContains(t, capped, lost, "the want is past the per-cycle cap") // A local part now covers the blockers, so they discharge without a fetch and the want is asked. - e.SetPartBlocks(e.PartPrefixes()[0], bucketindex.Interval{Min: 100, Max: 103}, 1) + e.SetPartBlocks(e.PartPrefixes()[0], bucketindex.Range(0, 100, 103), 1) mergeTimes(t, e, 1) holes := e.Holes() @@ -158,7 +158,7 @@ func repairConcurrentMergesFetchOnce(t *testing.T, k Kind) { CopyObjects(t, be, peer, lost) dropObjects(t, be, lost) - e.LosePart(lost, bucketindex.Interval{Min: 1, Max: 1}) + e.LosePart(lost, bucketindex.Range(0, 1, 1)) entered := make(chan struct{}, 2) release := make(chan struct{}) @@ -214,7 +214,7 @@ func (k Kind) seedGoneParts(t *testing.T, be backend.Backend, n int) []string { start, end := lostWindow(i) ix.Add(bucketindex.Entry{ Prefix: prefix, MinTime: start, MaxTime: end, - Blocks: bucketindex.Interval{Min: uint64(i + 1), Max: uint64(i + 1)}, + Blocks: bucketindex.Range(0, uint64(i+1), uint64(i+1)), }) lost = append(lost, prefix) } diff --git a/internal/repair/engine_test.go b/internal/repair/engine_test.go index 2ef8fb6b..45ce8ef4 100644 --- a/internal/repair/engine_test.go +++ b/internal/repair/engine_test.go @@ -106,7 +106,7 @@ func TestDriveCommitsGroupAndRetiresAncestor(t *testing.T) { t.Parallel() ancestor := bucketindex.Entry{Prefix: "other", Blocks: bucketindex.Blocks(2)} - members := group("f", 10, 3, bucketindex.Interval{Min: 1, Max: 2}) + members := group("f", 10, 3, bucketindex.Range(0, 1, 2)) h := &host{ wants: []bucketindex.Want{want("lost", 1)}, parts: []*fakePart{{ent: ancestor}}, diff --git a/internal/repair/group_test.go b/internal/repair/group_test.go index e9228c90..7351c911 100644 --- a/internal/repair/group_test.go +++ b/internal/repair/group_test.go @@ -93,11 +93,11 @@ func TestSupersededResultIsNotPublished(t *testing.T) { members := group("f", 10, 3, bucketindex.Blocks(1)) // Between rounds the peer merged f00 and f01; the output keeps the group's unrealized claim. merged := bucketindex.Entry{ - Prefix: "t", Blocks: bucketindex.Interval{Min: 10, Max: 11}, Claim: members[0].Claim, Level: 2, + Prefix: "t", Blocks: bucketindex.Range(0, 10, 11), Claim: members[0].Claim, Level: 2, } p := &peer{ ix: bucketindex.Index{Entries: members}, - member: map[uint64]bucketindex.FetchResult{11: {Entry: merged}}, + member: map[bucketindex.Block]bucketindex.FetchResult{{N: 11}: {Entry: merged}}, } plan := Pass{Fetcher: p}.Run(context.Background(), nil, []bucketindex.Want{want("lost", 1)}, nil) @@ -115,7 +115,7 @@ func TestSupersededResultIsNotPublished(t *testing.T) { func TestSharedGroupCountsEachPartOnce(t *testing.T) { t.Parallel() - members := group("f", 10, 3, bucketindex.Interval{Min: 1, Max: 2}) + members := group("f", 10, 3, bucketindex.Range(0, 1, 2)) p := &peer{ix: bucketindex.Index{Entries: members}} plan := Pass{Fetcher: p}.Run(context.Background(), nil, diff --git a/internal/repair/repair.go b/internal/repair/repair.go index c1a05f3f..e2c34080 100644 --- a/internal/repair/repair.go +++ b/internal/repair/repair.go @@ -244,12 +244,12 @@ func (p Pass) fetch(ctx context.Context, targets []Target, stats *bucketindex.Re // complete runs the member rounds: each asks, in one fetcher call, for every block some unit's split // groups still lack. A block is asked for once per pass however many units need it. func (p Pass) complete(ctx context.Context, ix *bucketindex.Index, units []*unit, stats *bucketindex.RepairStats) { - answers := make(map[uint64]attempt) + answers := make(map[bucketindex.Block]attempt) for { var ( ask []Target - askedBy = make(map[uint64][]*unit) + askedBy = make(map[bucketindex.Block][]*unit) progress bool ) @@ -271,7 +271,7 @@ func (p Pass) complete(ctx context.Context, ix *bucketindex.Index, units []*unit if _, dup := askedBy[b]; !dup { w := u.results[0].Want ask = append(ask, Target{ - Want: bucketindex.Want{Blocks: bucketindex.Blocks(b), MinTime: w.MinTime, MaxTime: w.MaxTime}, + Want: bucketindex.Want{Blocks: bucketindex.Single(b), MinTime: w.MinTime, MaxTime: w.MaxTime}, Member: true, }) } @@ -318,13 +318,13 @@ func (o outcome) want() bucketindex.WantOutcome { // unit is a [Unit] under construction. type unit struct { results []Result - asked map[uint64]struct{} + asked map[bucketindex.Block]struct{} // worst is why the unit can no longer complete this pass; outcomeNone while it still can. worst outcome } func newUnit(r Result) *unit { - return &unit{results: []Result{r}, asked: make(map[uint64]struct{})} + return &unit{results: []Result{r}, asked: make(map[bucketindex.Block]struct{})} } func (u *unit) entries(ix *bucketindex.Index) []bucketindex.Entry { @@ -339,14 +339,14 @@ func (u *unit) entries(ix *bucketindex.Index) []bucketindex.Entry { // missing is the member blocks the unit still needs and has not asked for: every group of its parts // while its target is not yet answered, and otherwise the groups whose lone members would duplicate // rows the index already holds ([groupsNeeded]). -func (u *unit) missing(ix *bucketindex.Index) []uint64 { +func (u *unit) missing(ix *bucketindex.Index) []bucketindex.Block { entries := u.entries(ix) held := (&bucketindex.Index{Entries: entries}).Covered() - out := make(map[uint64]struct{}) + out := make(map[bucketindex.Block]struct{}) for _, c := range groupsNeeded(ix.Entries, u.results[0].Want, entries[len(ix.Entries):]) { - c.Group.Each(func(b uint64) bool { - if _, asked := u.asked[b]; !asked && !held.Contains(bucketindex.Blocks(b)) { + c.Group.Each(func(b bucketindex.Block) bool { + if _, asked := u.asked[b]; !asked && !held.Contains(bucketindex.Single(b)) { out[b] = struct{}{} } @@ -354,7 +354,7 @@ func (u *unit) missing(ix *bucketindex.Index) []uint64 { }) } - return slices.Sorted(maps.Keys(out)) + return slices.SortedFunc(maps.Keys(out), bucketindex.Block.Compare) } func (u *unit) take(a attempt) { diff --git a/internal/repair/repair_test.go b/internal/repair/repair_test.go index 6bb3360e..669aebd3 100644 --- a/internal/repair/repair_test.go +++ b/internal/repair/repair_test.go @@ -19,7 +19,7 @@ import ( type peer struct { ix bucketindex.Index // member overrides the answer to a member want for a block. - member map[uint64]bucketindex.FetchResult + member map[bucketindex.Block]bucketindex.FetchResult calls [][]bucketindex.Want // disk, when set, receives every part the peer answers with, as a copy would. disk map[string]bucketindex.Entry @@ -70,7 +70,7 @@ func want(prefix string, b uint64) bucketindex.Want { // group returns the n members of a split group over run [first, first+n) jointly claiming claimed. func group(name string, first, n uint64, claimed bucketindex.Interval) []bucketindex.Entry { - c := bucketindex.Claim{Blocks: claimed, Group: bucketindex.Interval{Min: first, Max: first + n - 1}} + c := bucketindex.Claim{Blocks: claimed, Group: bucketindex.Range(0, first, first+n-1)} out := make([]bucketindex.Entry, n) for i := range n { @@ -94,7 +94,7 @@ func prefixes(u Unit) []string { func TestRunLocal(t *testing.T) { t.Parallel() - local := bucketindex.Entry{Prefix: "succ", Blocks: bucketindex.Interval{Min: 1, Max: 2}, Level: 1} + local := bucketindex.Entry{Prefix: "succ", Blocks: bucketindex.Range(0, 1, 2), Level: 1} p := &peer{} plan := Pass{Fetcher: p}.Run(context.Background(), []bucketindex.Entry{local}, @@ -221,7 +221,7 @@ func TestRunCompletesWideGroup(t *testing.T) { func TestRunSharedGroupAskedOnce(t *testing.T) { t.Parallel() - members := group("f", 10, 6, bucketindex.Interval{Min: 1, Max: 2}) + members := group("f", 10, 6, bucketindex.Range(0, 1, 2)) p := &peer{ix: bucketindex.Index{Entries: members}} plan := Pass{Fetcher: p}.Run(context.Background(), nil, @@ -281,7 +281,7 @@ func TestRunShortGroup(t *testing.T) { members := group("f", 10, 8, bucketindex.Blocks(1)) p := &peer{ ix: bucketindex.Index{Entries: members}, - member: map[uint64]bucketindex.FetchResult{17: tc.member}, + member: map[bucketindex.Block]bucketindex.FetchResult{{N: 17}: tc.member}, } plan := Pass{Fetcher: p}.Run(context.Background(), nil, @@ -402,7 +402,7 @@ func TestAdmit(t *testing.T) { var stats bucketindex.RepairStats live := []bucketindex.Entry{members[0]} - succ := bucketindex.Entry{Prefix: "s", Blocks: bucketindex.Interval{Min: 1, Max: 2}, Level: 2} + succ := bucketindex.Entry{Prefix: "s", Blocks: bucketindex.Range(0, 1, 2), Level: 2} phantom := bucketindex.Entry{Prefix: "x", Blocks: bucketindex.Blocks(2)} open, opened := opener("x") @@ -425,8 +425,8 @@ func TestAdmit(t *testing.T) { t.Run("PartialSuccessor", func(t *testing.T) { t.Parallel() - wide := group("g", 20, 2, bucketindex.Interval{Min: 1, Max: 2}) - part := bucketindex.Entry{Prefix: "p", Blocks: bucketindex.Interval{Min: 2, Max: 3}, Level: 2} + wide := group("g", 20, 2, bucketindex.Range(0, 1, 2)) + part := bucketindex.Entry{Prefix: "p", Blocks: bucketindex.Range(0, 2, 3), Level: 2} units := []Unit{ unit("a", false, false, wide...), unitOf(want("b", 3), false, false, part), @@ -461,7 +461,7 @@ func TestAdmit(t *testing.T) { var stats bucketindex.RepairStats - rival := bucketindex.Entry{Prefix: "r", Blocks: bucketindex.Interval{Min: 1, Max: 2}, Level: 1} + rival := bucketindex.Entry{Prefix: "r", Blocks: bucketindex.Range(0, 1, 2), Level: 1} open, _ := opener() got, _ := Admit(ctx, members[:1], []Unit{ unit("a", false, false, members...), diff --git a/recordengine/ARCH.md b/recordengine/ARCH.md index 9940730a..4b63a1c8 100644 --- a/recordengine/ARCH.md +++ b/recordengine/ARCH.md @@ -440,8 +440,10 @@ serves stay the same set; the adopted parts are readable but not owned. See **Block identity** is allocated the same way: a flush output takes a fresh `{n}` at level 0, a merge that writes one part the union of the block sets its inputs covered at one level above them, and a merge that splits a fresh contiguous run plus the joint `bucketindex.Claim` over it. Numbering runs -above the index's persisted high-water mark, and every assignment is made per CAS attempt and -written onto the part only once the commit lands. See [`../engine/ARCH.md`](../engine/ARCH.md), +above the index's persisted high-water mark within the committing tenure's term, and every +assignment is made per CAS attempt and written onto the part only once the commit lands. The +planning and allocation are the metric engine's (`bucketindex.Plan`), and so is the tenure fence on +every commit ([`../engine/ARCH.md`](../engine/ARCH.md), "A clustered commit is fenced by tenure"). See [`../engine/ARCH.md`](../engine/ARCH.md), "Block identity is allocated by the commit that publishes the part". **Repair** is identical to the metric engine, down to `Config.Repair` and its `bucketindex` types, the satisfaction rule (the diff --git a/recordengine/blockid.go b/recordengine/blockid.go index d05daa45..89962e55 100644 --- a/recordengine/blockid.go +++ b/recordengine/blockid.go @@ -6,30 +6,9 @@ import ( "github.com/oteldb/storage/backend/bucketindex" ) -// A part's identity in block-number space is assigned by the commit that publishes it, out of the -// index that commit is building. Allocation is the shard owner's alone and needs no coordination: -// the CAS that adds the part is what claims the number, so a writer that loses the race re-reads -// and allocates above the winner. - -// blockPlan is the identity a newly written part awaits. A flush leaves blocks unset, so the -// commit allocates a fresh number for it; a merge that writes one part inherits the set of blocks -// its inputs covered, which is what makes the output supersede them. -type blockPlan struct { - blocks bucketindex.Interval - claim bucketindex.Claim - // group is set on the outputs of a merge that split, and shared by all of them: the commit - // allocates the whole run at once so the fragments can name each other. - group *splitGroup - level uint32 -} - -// splitGroup is the joint claim the outputs of one split merge carry. The ancestor blocks are -// known when the merge is planned; the group's own blocks are not, because allocation happens per -// commit attempt — see [Engine.nextIndexLocked]. -type splitGroup struct { - blocks bucketindex.Interval - n int -} +// A part's identity in block space is assigned by the commit that publishes it, out of the index +// that commit is building, under the tenure it writes as: see [bucketindex.Plan] and +// [bucketindex.Allocator], which both engines share. // blockAssignment is an identity [Engine.nextIndexLocked] chose while building one candidate // index. It is written onto the part only by the commit that lands: a part numbered before its @@ -40,99 +19,23 @@ type blockAssignment struct { blocks bucketindex.Interval claim bucketindex.Claim level uint32 + term uint64 } // planFlushBlocks marks p as a flush output: a fresh block at level 0. -func planFlushBlocks(p *part) { p.pending = &blockPlan{} } +func planFlushBlocks(p *part) { p.pending = bucketindex.FlushPlan() } -// planMergeBlocks stamps the identity the outputs of a merge over src claim. -// -// A merge that writes one part does not allocate. The output covers exactly the blocks its inputs -// covered — a set, so a run that straddles a gap claims no block in it — one level above the -// deepest input, and that is precisely what makes supersession decidable from identity alone, so a -// repair can accept the successor of the part it wanted. -// -// A merge that splits its output cannot hand any fragment that set: none holds all of the data, so -// a successor claim would answer a want with a fraction of the part. The fragments take a fresh run -// of blocks instead and carry the inputs' set as a joint [bucketindex.Claim] over that run — which -// is what keeps the lineage: the claim is realized wherever every member of the run is present, and -// a later merge that consumes them all folds it back into an ordinary interval. -// -// A merge whose inputs all predate format v5 has no set to inherit; a fresh block is how such a -// part migrates, a merge being the only thing that rewrites it. A mixed merge — some inputs -// carrying an interval, some not — inherits the union of the ones that do. It cannot do better: a -// want naming a pre-v5 part records that part's unset interval, so no claim the output makes could -// contain it. -// -// One limit is deliberate: an output carries at most one unrealized claim, the widest, so a merge -// consuming fragments of two groups without completing either keeps the lineage of one. The other -// falls back to exact-prefix matching, which is where every split left it before claims existed. +// planMergeBlocks stamps the identity the outputs of a merge over src claim: see +// [bucketindex.PlanMerge]. func planMergeBlocks(src, out []*part) { - var ( - union bucketindex.Interval - level uint32 - claims []bucketindex.Claim - ) - - for _, p := range src { - level = max(level, p.level+1) - union = union.Union(p.blocks) - - if p.claim.Valid() { - claims = append(claims, p.claim) - } - } - - union, claims = realizeClaims(union, claims) - - if len(out) == 1 { - out[0].pending = &blockPlan{blocks: union, claim: widestClaim(claims), level: level} - - return - } - - group := &splitGroup{blocks: union, n: len(out)} - for _, p := range out { - p.pending = &blockPlan{group: group, level: level} - } -} - -// realizeClaims folds into held every claim whose group is wholly inside it, repeating while that -// keeps realizing more — a group whose members were themselves split resolves only after the inner -// one has. It returns what is covered and the claims still owed a member. -func realizeClaims(held bucketindex.Interval, claims []bucketindex.Claim) (bucketindex.Interval, []bucketindex.Claim) { - for len(claims) > 0 { - rest := claims[:0] - - for _, c := range claims { - if held.Contains(c.Group) { - held = held.Union(c.Blocks) - - continue - } - - rest = append(rest, c) - } - - if len(rest) == len(claims) { - break - } - - claims = rest + ents := make([]bucketindex.Entry, len(src)) + for i, p := range src { + ents[i] = bucketindex.Entry{Blocks: p.blocks, Claim: p.claim, Level: p.level} } - return held, claims -} - -func widestClaim(claims []bucketindex.Claim) bucketindex.Claim { - var best bucketindex.Claim - for _, c := range claims { - if c.Blocks.Len() > best.Blocks.Len() { - best = c - } + for i, plan := range bucketindex.PlanMerge(ents, len(out)) { + out[i].pending = plan } - - return best } // nextBlockLocked is [bucketindex.Index.NextBlock] over the whole state the commit under @@ -140,7 +43,7 @@ func widestClaim(claims []bucketindex.Claim) bucketindex.Claim { // writer's included, since its blocks are real claims and reusing one would make two different // parts share an identity — this engine's own parts, the holes it carries, and every want // outstanding or pending, whose part may yet be repaired back into the index. Caller holds e.mu. -func (e *Engine) nextBlockLocked(ix *bucketindex.Index) uint64 { +func (e *Engine) nextBlockLocked(ix *bucketindex.Index) bucketindex.Block { seed := bucketindex.Index{ Entries: slices.Concat(ix.Entries, e.holes), Wanted: slices.Concat(e.wants, e.pendingWants, e.adoptedWants, e.pendingHoles), @@ -151,51 +54,5 @@ func (e *Engine) nextBlockLocked(ix *bucketindex.Index) uint64 { seed.Entries = append(seed.Entries, bucketindex.Entry{Blocks: p.blocks, Claim: p.claim}) } - return seed.NextBlock() -} - -// groupRun is the run of blocks one commit attempt allocated to a split group, and how far into it -// the fragments assigned so far have got. -type groupRun struct { - run bucketindex.Interval - next uint64 -} - -// allocateBlocks turns a pending plan into a real identity, taking numbers from *next. -// -// A split group takes its whole run at once, before any of its fragments is numbered, so every -// fragment carries the same claim and can name the siblings a repair still has to fetch. -func allocateBlocks( - id *blockPlan, next *uint64, groups map[*splitGroup]*groupRun, -) (bucketindex.Interval, bucketindex.Claim) { - if id.blocks.Valid() { - return id.blocks, id.claim - } - - if id.group == nil { - iv := bucketindex.Interval{Min: *next, Max: *next} - *next++ - - return iv, id.claim - } - - g, ok := groups[id.group] - if !ok { - g = &groupRun{ - run: bucketindex.Interval{Min: *next, Max: *next + uint64(id.group.n) - 1}, - next: *next, - } - groups[id.group] = g - *next = g.run.Max + 1 - } - - iv := bucketindex.Interval{Min: g.next, Max: g.next} - g.next++ - - claim := bucketindex.Claim{Blocks: id.group.blocks, Group: g.run} - if !claim.Valid() { - return iv, bucketindex.Claim{} - } - - return iv, claim + return seed.NextBlock(e.term()) } diff --git a/recordengine/engine.go b/recordengine/engine.go index 6cbf2bec..dc374f7e 100644 --- a/recordengine/engine.go +++ b/recordengine/engine.go @@ -255,10 +255,10 @@ type Engine struct { // lostParts is the index's monotone data-loss counter, carried across commits and raised to a // rival's on rebase, so it is a cluster-visible fact and not a level a restart clears. lostParts uint64 - // allocated is the block-number high-water mark this engine last committed, carried across - // commits and raised to a rival's on rebase for the same reason: identity must stay unique over - // the shard's whole life, and the live set it could otherwise be derived from shrinks. - allocated uint64 + // allocated is the block high-water mark this engine last committed, carried across commits + // and raised to a rival's on rebase for the same reason: identity must stay unique over the + // shard's whole life, and the live set it could otherwise be derived from shrinks. + allocated bucketindex.Block // pendingHoles are the losses the next commit must acknowledge. Like pendingWants they are // held rather than applied on the spot, so a commit that never lands leaves the want // outstanding instead of half-discharged. diff --git a/recordengine/export_test.go b/recordengine/export_test.go index 50424369..76663590 100644 --- a/recordengine/export_test.go +++ b/recordengine/export_test.go @@ -124,7 +124,7 @@ func (e *Engine) LoadState() any { Indexed map[string]struct{} Holes []bucketindex.Entry LostParts uint64 - Allocated uint64 + Allocated bucketindex.Block IdentityDirty bool FlushedEpoch uint64 Epochs []bucketindex.WriterEpoch diff --git a/recordengine/index.go b/recordengine/index.go index b74ef496..dbe412c9 100644 --- a/recordengine/index.go +++ b/recordengine/index.go @@ -40,6 +40,14 @@ const indexCommitAttempts = 8 // part is dropped from the index that survives. It is a no-op for a head-only engine (no // backend), and refused while the engine is fenced ([Engine.fenceLocked]). Caller holds e.mu. func (e *Engine) updateIndexLocked(ctx context.Context) error { + return e.commitIndexLocked(ctx, e.term()) +} + +// commitIndexLocked is [Engine.updateIndexLocked] for an operation that began under ownership term +// stamp. Every attempt first passes the tenure fence ([Engine.tenureLocked]), so a writer displaced +// while the operation ran — or one a rebase shows a later tenure has overtaken — commits nothing. +// Caller holds e.mu. +func (e *Engine) commitIndexLocked(ctx context.Context, stamp uint64) error { if e.cfg.Backend == nil { return nil } @@ -49,6 +57,10 @@ func (e *Engine) updateIndexLocked(ctx context.Context) error { } for range indexCommitAttempts { + if err := e.tenureLocked(stamp); err != nil { + return err + } + ix := e.nextIndexLocked(ctx) version, err := ix.Save(ctx, e.cfg.Backend, e.indexKey(), e.indexVersion) @@ -62,7 +74,8 @@ func (e *Engine) updateIndexLocked(ctx context.Context) error { // landed would hold a block the winner took, and the retry would not re-allocate. for i := range e.pendingBlocks { a := &e.pendingBlocks[i] - a.part.blocks, a.part.claim, a.part.level, a.part.pending = a.blocks, a.claim, a.level, nil + a.part.blocks, a.part.claim, a.part.level, a.part.term = a.blocks, a.claim, a.level, a.term + a.part.pending = nil } e.pendingBlocks = nil @@ -137,7 +150,7 @@ func (e *Engine) adoptIndexLocked(ctx context.Context) error { // not walk back: the counter only ever rises. e.holes = mergeHoles(e.holes, ix.Holes()) e.lostParts = max(e.lostParts, ix.LostParts) - e.allocated = max(e.allocated, ix.AllocatedBlocks) + e.allocated = bucketindex.MaxBlock(e.allocated, ix.AllocatedBlocks) e.foreign = foreignEntries(ix.Entries, e.indexed, e.removals, e.wants) e.openForeignLocked(ctx) @@ -250,32 +263,36 @@ func (e *Engine) nextIndexLocked(ctx context.Context) *bucketindex.Index { // Block numbers are allocated here, per attempt, because a rival's entries are only known // after a rebase: allocating once and reusing it across retries would hand a part a block the // winner already claimed. The assignments are held, not applied — see [Engine.updateIndexLocked]. - next := e.nextBlockLocked(ix) - groups := make(map[*splitGroup]*groupRun) + alloc := bucketindex.NewAllocator(e.nextBlockLocked(ix)) var assigned []blockAssignment + own := make([]bucketindex.Entry, 0, len(e.parts)) live := make(map[string]struct{}, len(e.parts)) + for _, p := range e.parts { - blocks, claim, level := p.blocks, p.claim, p.level - if id := p.pending; id != nil { - blocks, claim = allocateBlocks(id, &next, groups) - level = id.level + blocks, claim, level, term := p.blocks, p.claim, p.level, p.term + if plan := p.pending; plan != nil { + blocks, claim = alloc.Assign(plan) + level, term = plan.Level, alloc.Term() - assigned = append(assigned, blockAssignment{part: p, blocks: blocks, claim: claim, level: level}) + assigned = append(assigned, blockAssignment{part: p, blocks: blocks, claim: claim, level: level, term: term}) } - ix.Add(bucketindex.Entry{ + ent := bucketindex.Entry{ Prefix: p.prefix, MinTime: p.minTime, MaxTime: p.maxTime, - Blocks: blocks, Claim: claim, Level: level, - }) + Blocks: blocks, Claim: claim, Level: level, Term: term, + } + ix.Add(ent) + own = append(own, ent) live[p.prefix] = struct{}{} } - // One above the last block handed out, over every source this attempt considered — so the mark - // only ever rises, and a shard whose live set has since emptied never renumbers over a part it - // once held. - ix.AllocatedBlocks = next - 1 + e.dropSubsumedForeignLocked(ix, own) + + // The last block handed out, over every source this attempt considered — so the mark only ever + // rises, and a shard whose live set has since emptied never renumbers over a part it once held. + ix.AllocatedBlocks = alloc.Mark() e.pendingBlocks = assigned @@ -469,7 +486,7 @@ func (e *Engine) livePart( delete(open, ent.Prefix) p.minTime, p.maxTime = ent.MinTime, ent.MaxTime - p.blocks, p.claim, p.level = ent.Blocks, ent.Claim, ent.Level + p.blocks, p.claim, p.level, p.term = ent.Blocks, ent.Claim, ent.Level, ent.Term return p, nil } @@ -478,7 +495,7 @@ func (e *Engine) livePart( // still pending, so reusing it needs no write. func (p *part) matchesEntry(ent *bucketindex.Entry) bool { return p.pending == nil && p.minTime == ent.MinTime && p.maxTime == ent.MaxTime && - p.level == ent.Level && p.blocks.Equal(ent.Blocks) && p.claim.Equal(ent.Claim) + p.level == ent.Level && p.term == ent.Term && p.blocks.Equal(ent.Blocks) && p.claim.Equal(ent.Claim) } // indexLoad is the state a load replaces, read and opened into locals so that a load failing @@ -727,8 +744,9 @@ func (e *Engine) loadPartsLocked(ctx context.Context, mode loadMode) error { // One commit drops the gone parts from Entries and states the wants that replace them. It runs // before the rest of the load so the obligation is durable even if identity recovery fails. A // carried part's identity would be stamped by this commit, which does not hold flushMu, so the - // wants then stay pending for the next flush or merge to commit. - if len(l.lost) > 0 && mode == loadOwner && !l.carried { + // wants then stay pending for the next flush or merge to commit. So do they on a clustered engine + // without the shard's claim, which may not commit at all: the load is then an unclaimed one. + if len(l.lost) > 0 && mode == loadOwner && !l.carried && e.tenureLocked(e.term()) == nil { if err := e.updateIndexLocked(ctx); err != nil { return errors.Wrap(err, "record repair wants") } @@ -884,3 +902,41 @@ func decodeSeriesSet(data []byte, fn func(signal.Series)) error { return nil } + +// tenureLocked is the commit fence of a clustered writer ([bucketindex.CheckTenure]) for an +// operation that began under term stamp, checked against the term of the index the commit builds on. +// A writer with no cluster has no tenure to lose. Caller holds e.mu. +func (e *Engine) tenureLocked(stamp uint64) error { + if e.cfg.Term == nil { + return nil + } + + return bucketindex.CheckTenure(stamp, e.term(), e.generation.Term) +} + +// dropSubsumedForeignLocked takes out of ix, and tombstones, the adopted entries whose every row this +// engine's own entries hold — the same merge a displaced tenure committed before this one, resolved +// to the later tenure, or a part a merge here has since consumed. Carrying one forward would keep a +// set of rows live twice. Caller holds e.mu. +func (e *Engine) dropSubsumedForeignLocked(ix *bucketindex.Index, own []bucketindex.Entry) { + if len(e.foreign) == 0 { + return + } + + gone := bucketindex.Subsumed(e.foreign, own) + if len(gone) == 0 { + return + } + + for prefix := range gone { + ix.Remove(prefix) + delete(e.foreignParts, prefix) + e.removals = append(e.removals, bucketindex.Removal{Prefix: prefix, Generation: e.generation}) + } + + e.foreign = slices.DeleteFunc(e.foreign, func(ent bucketindex.Entry) bool { + _, ok := gone[ent.Prefix] + + return ok + }) +} diff --git a/recordengine/merge.go b/recordengine/merge.go index 981fc5a7..5fef5fff 100644 --- a/recordengine/merge.go +++ b/recordengine/merge.go @@ -128,6 +128,9 @@ func (e *Engine) mergeHeld(ctx context.Context, opts MergeOptions, abandoned *[] e.flushMu.Lock() defer e.flushMu.Unlock() + // The tenure the inputs were chosen under: a merge that outlives it commits nothing. + stamp := e.term() + // Plan (under lock): snapshot the source parts (immutable backing). Output part ids are minted one // at a time, as the parts are written. e.mu.Lock() @@ -219,7 +222,7 @@ func (e *Engine) mergeHeld(ctx context.Context, opts MergeOptions, abandoned *[] committed := e.parts e.parts = replaceParts(e.parts, removed, newParts...) - if err = e.updateIndexLocked(ctx); err != nil { + if err = e.commitIndexLocked(ctx, stamp); err != nil { e.parts = committed e.mu.Unlock() diff --git a/recordengine/mergewriter_internal_test.go b/recordengine/mergewriter_internal_test.go index 07661fb0..8db669f0 100644 --- a/recordengine/mergewriter_internal_test.go +++ b/recordengine/mergewriter_internal_test.go @@ -435,14 +435,14 @@ func TestPlanMergeBlocksSameStreamBothSides(t *testing.T) { union = union.Union(p.blocks) } - group := out[0].pending.group + group := out[0].pending.Group require.NotNil(t, group) - assert.Equal(t, len(out), group.n) - assert.Equal(t, union, group.blocks) + assert.Equal(t, len(out), group.N) + assert.Equal(t, union, group.Blocks) for _, p := range out { - assert.Same(t, group, p.pending.group, "one joint claim for every fragment") - assert.False(t, p.pending.blocks.Valid(), "a fragment inherits no block of its own") + assert.Same(t, group, p.pending.Group, "one joint claim for every fragment") + assert.False(t, p.pending.Blocks.Valid(), "a fragment inherits no block of its own") } // The same shape through the engine: fragments committed, each with a fresh block and the joint @@ -468,7 +468,7 @@ func TestPlanMergeBlocksSameStreamBothSides(t *testing.T) { var claims []string - blocks := map[uint64]bool{} + blocks := map[bucketindex.Block]bool{} for _, p := range e2.parts { if p.level == 0 { diff --git a/recordengine/part.go b/recordengine/part.go index 23683d0d..4f377a04 100644 --- a/recordengine/part.go +++ b/recordengine/part.go @@ -67,11 +67,13 @@ type part struct { // claim is the ancestry this part holds only jointly with the rest of its split group; unset // for every part a merge wrote whole — see [bucketindex.Claim]. claim bucketindex.Claim + // term is the ownership term of the tenure that wrote the part ([bucketindex.Entry.Term]). + term uint64 // pending is the identity a part this engine just wrote is waiting to be assigned; nil for one // opened from an index, whose identity is whatever that index recorded — including the unset // one of a part written before format v5. See blockid.go. - pending *blockPlan + pending *bucketindex.Plan // rawBytes is the part's decoded footprint per its manifest, 0 for a part written before the // manifest carried one. See [part.sizeBytes]. diff --git a/recordengine/repair.go b/recordengine/repair.go index 65fd7208..f41a5b08 100644 --- a/recordengine/repair.go +++ b/recordengine/repair.go @@ -64,7 +64,7 @@ func (h repairHost) Reclaim(ctx context.Context) { h.e.reclaimRet func (h repairHost) Identity(p *part) bucketindex.Entry { return bucketindex.Entry{ Prefix: p.prefix, MinTime: p.minTime, MaxTime: p.maxTime, - Blocks: p.blocks, Claim: p.claim, Level: p.level, + Blocks: p.blocks, Claim: p.claim, Level: p.level, Term: p.term, } } @@ -74,7 +74,7 @@ func (h repairHost) Open(ctx context.Context, prefix string) (*part, error) { func (h repairHost) AdoptLocked(ctx context.Context, p *part, ent *bucketindex.Entry) error { p.minTime, p.maxTime = ent.MinTime, ent.MaxTime - p.blocks, p.claim, p.level = ent.Blocks, ent.Claim, ent.Level + p.blocks, p.claim, p.level, p.term = ent.Blocks, ent.Claim, ent.Level, ent.Term _, err := h.e.registerPartIdentitiesLocked(ctx, ent.Prefix) diff --git a/recordengine/splitlineage_test.go b/recordengine/splitlineage_test.go index 0f05efd5..64576c71 100644 --- a/recordengine/splitlineage_test.go +++ b/recordengine/splitlineage_test.go @@ -124,7 +124,7 @@ func flushInputs(ctx context.Context, t *testing.T, be backend.Backend, split in for i, id := range ids { ent := byPrefix[enginePrefix+"/"+id] - require.Equal(t, bucketindex.Interval{Min: uint64(i + 1), Max: uint64(i + 1)}, ent.Blocks) + require.Equal(t, bucketindex.Range(0, uint64(i+1), uint64(i+1)), ent.Blocks) out = append(out, ent) } @@ -173,7 +173,7 @@ func TestSplitMergeAllocatesAboveItsInputs(t *testing.T) { _, fragments := splitMerge(ctx, t, be, inputs) for _, f := range fragments { - assert.Greater(t, f.Blocks.Min, inputs[2].Blocks.Max, "fragment %+v is numbered below a retired input", f) + assert.Positive(t, f.Blocks.Min.Compare(inputs[2].Blocks.Max), "fragment %+v is numbered below a retired input", f) for _, in := range inputs { assert.False(t, f.Supersedes(in), "fragment %+v holds a fraction of %+v and must not claim it", f, in) From 4c6a1f220e7272da5bc55f2ba5501f408cd10e38 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Mon, 28 Sep 2026 21:49:33 +0300 Subject: [PATCH 2/8] test(bucketindex): pin the inverted v5 hull the decode fuzzer found Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KcLq1G8KpWBvM53JaQb1tS --- backend/bucketindex/testdata/fuzz/FuzzDecode/6fdd6985b2c286ac | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 backend/bucketindex/testdata/fuzz/FuzzDecode/6fdd6985b2c286ac diff --git a/backend/bucketindex/testdata/fuzz/FuzzDecode/6fdd6985b2c286ac b/backend/bucketindex/testdata/fuzz/FuzzDecode/6fdd6985b2c286ac new file mode 100644 index 00000000..210f30d2 --- /dev/null +++ b/backend/bucketindex/testdata/fuzz/FuzzDecode/6fdd6985b2c286ac @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("BI\x05\x00000\x02\x0000\x0000\x00\x01\x06000000100000000") From cd5fe568258a0e40868284f6388b7a4e349a86c9 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Mon, 28 Sep 2026 22:10:12 +0300 Subject: [PATCH 3/8] fix(storage): flush owned engines before a clustered Close leaves the cluster A flush without the claim is now refused before it writes anything, and an engine Close takes that refusal as nothing to flush, so a node that left the cluster closes cleanly with its head in the WAL. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KcLq1G8KpWBvM53JaQb1tS --- cluster/ARCH.md | 2 + cluster_close_test.go | 67 ++++++++++++++++++++++++++++++ engine/ARCH.md | 8 ++-- engine/engine.go | 10 ++++- engine/merge.go | 5 ++- internal/enginetest/tenurefence.go | 13 +++--- recordengine/engine.go | 10 ++++- recordengine/merge.go | 5 ++- storage.go | 18 ++++++++ 9 files changed, 124 insertions(+), 14 deletions(-) create mode 100644 cluster_close_test.go diff --git a/cluster/ARCH.md b/cluster/ARCH.md index 4508c3b7..4dfe49b8 100644 --- a/cluster/ARCH.md +++ b/cluster/ARCH.md @@ -78,6 +78,8 @@ term its operation began under, or once the index it builds on carries a higher (`backend/ARCH.md`, *Part identity*). A handoff therefore never stalls a shard: the new owner flushes and merges with the freshest view it has. The cost is duplicate rows, never a collision — an old owner's last flush that no replica mirrored is re-flushed by the new owner from its replica head. +`Storage.Close` therefore flushes every engine it owns *before* leaving the cluster, while the claims +still hold; after leaving, the engines' final flushes are refused and their heads stay in the WAL. Tests boot etcd through **`cluster/etcd/etcdtest`**, which binds its listeners to port 0 and reads the client port back from the socket. Probing a free port and releasing it for etcd to bind is diff --git a/cluster_close_test.go b/cluster_close_test.go new file mode 100644 index 00000000..18c11051 --- /dev/null +++ b/cluster_close_test.go @@ -0,0 +1,67 @@ +package storage + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/oteldb/storage/backend/file" + "github.com/oteldb/storage/cluster" + "github.com/oteldb/storage/cluster/etcd" + "github.com/oteldb/storage/cluster/etcd/etcdtest" + "github.com/oteldb/storage/signal" +) + +// TestClusterCloseFlushesWhileTheClaimIsHeld pins the order of a clustered Close: an owner drains +// its head while it still holds the shard's claim, since once it has left the cluster no commit of +// its is a tenure's. A node that does not own a shard closes cleanly too, leaving that head to the +// WAL. +// +//nolint:paralleltest // owns an embedded etcd; runs serially +func TestClusterCloseFlushesWhileTheClaimIsHeld(t *testing.T) { + endpoint := etcdtest.Start(t) + ctx := context.Background() + dataDir, walDir := t.TempDir(), t.TempDir() + + open := func() *Storage { + t.Helper() + + be, err := file.New(dataDir) + require.NoError(t, err) + + s, err := Open(ctx, Options{}, WithBackend(be), WithWALDir(walDir), WithFlushInterval(-1), + WithCluster(&cluster.Config{ + Etcd: []string{endpoint}, + Self: etcd.Member{ID: "node-a", Addr: "127.0.0.1:0"}, + RF: 1, + PrivateBackend: true, + })) + require.NoError(t, err) + + return s + } + + s := open() + _, err := s.WriteMetrics(ctx, gaugeBatch("api", "http.requests", []int64{100, 200}, []float64{1, 2})) + require.NoError(t, err) + + const shard = signal.TenantID("default") + + _, owned := s.ownedTenants(ctx, map[signal.TenantID]struct{}{shard: {}})[shard] + require.True(t, owned) + require.NoError(t, s.Close(ctx)) + + s = open() + t.Cleanup(func() { _ = s.Close(ctx) }) + + e, ok := s.lookupEngine(shard) + require.True(t, ok) + assert.Equal(t, 1, e.PartCount(), "the head was flushed before the node left the cluster") + assert.Zero(t, e.HeadSampleCount(), "and the WAL replays nothing a part already holds") + + _, err = s.WriteMetrics(ctx, gaugeBatch("api", "http.requests", []int64{300}, []float64{3})) + require.NoError(t, err) + require.NoError(t, s.Close(ctx), "a node holding no claim closes cleanly, its head left to the WAL") +} diff --git a/engine/ARCH.md b/engine/ARCH.md index 5e8f59fb..743b26ca 100644 --- a/engine/ARCH.md +++ b/engine/ARCH.md @@ -297,9 +297,11 @@ builds on (`generation.Term`, which a load or a rebase raises). A writer with no not fenced. A merge stamps its term when it picks its inputs: one whose tenure ended and restarted while it ran chose them from a view the intervening tenure may have merged differently. A flush, a retention drop and a repair commit take the term at commit instead, since what they publish is -this node's own head or a part it fetched, valid under any tenure the node holds. A refused flush -keeps its part as a carried, uncommitted part, like any failed commit; a refused merge rolls back -and leaves its output to the sweep. +this node's own head or a part it fetched, valid under any tenure the node holds. A flush checks +before it writes anything, so a refused one folds its rows back into the head (and its WAL); one +refused only at the commit keeps its part as a carried, uncommitted part, like any failed commit. A +refused merge rolls back and leaves its output to the sweep. `Close` treats the refusal as nothing to +flush: the head stays in the WAL for the next start. ## A failed load changes nothing, and fences every commit diff --git a/engine/engine.go b/engine/engine.go index 90951e4b..64aaad3f 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -1601,8 +1601,14 @@ func (e *Engine) flush(ctx context.Context) (rows int, written int64, err error) return 0, 0, nil } - // Checked after the detach so an empty head flushes as a no-op even while fenced. - if err := e.fenceLocked(); err != nil { + // Checked after the detach so an empty head flushes as a no-op even while fenced, and before any + // I/O so a writer without its tenure writes no part it could not commit. + err = e.fenceLocked() + if err == nil { + err = e.tenureLocked(e.term()) + } + + if err != nil { e.head.reattach(detached, detachedBytes, detachedSince) e.mu.Unlock() diff --git a/engine/merge.go b/engine/merge.go index f6b12b8f..aeef7e09 100644 --- a/engine/merge.go +++ b/engine/merge.go @@ -12,6 +12,7 @@ import ( "go.opentelemetry.io/otel/trace" "go.uber.org/zap" + "github.com/oteldb/storage/backend/bucketindex" "github.com/oteldb/storage/block" "github.com/oteldb/storage/internal/mergestream" "github.com/oteldb/storage/internal/timebucket" @@ -795,7 +796,9 @@ func mergeStreamedSeries( // Close flushes any buffered samples to a part and closes the WAL. It does not stop a background // loop — the owner ([storage.Storage]) does that before calling Close. func (e *Engine) Close(ctx context.Context) error { - if _, _, err := e.flush(ctx); err != nil { + // A clustered engine without the shard's claim leaves its head to the WAL, for the next start + // to replay, rather than failing to close. + if _, _, err := e.flush(ctx); err != nil && !errors.Is(err, bucketindex.ErrSuperseded) { return err } diff --git a/internal/enginetest/tenurefence.go b/internal/enginetest/tenurefence.go index 5b62c021..60013857 100644 --- a/internal/enginetest/tenurefence.go +++ b/internal/enginetest/tenurefence.go @@ -118,8 +118,9 @@ func writingTermSurvivesTheNextTenure(t *testing.T, k Kind) { } // lapsedClaimCommitsNothing is the commit fence over a writer's own term: a flush while the claim is -// gone is refused and keeps its rows, and a merge whose tenure ended and restarted while it ran is -// refused too, since it chose its inputs under a tenure that is over. +// gone is refused before it writes anything and keeps its rows in the head, and a merge whose tenure +// ended and restarted while it ran is refused too, since it chose its inputs under a tenure that is +// over. func lapsedClaimCommitsNothing(t *testing.T, k Kind) { t.Helper() @@ -139,12 +140,14 @@ func lapsedClaimCommitsNothing(t *testing.T, k Kind) { e.Append(t, api(300, 3)) require.ErrorIs(t, e.Flush(ctx), bucketindex.ErrSuperseded, "no claim, no commit") assert.Len(t, k.loadIndex(t, inner).Entries, 2) + assert.Len(t, k.partDirs(ctx, t, inner), 2, "and no part written for it") + assert.Equal(t, 1, e.HeadRows(), "the rows stay in the head") term.Store(3) e.Append(t, api(400, 4)) - require.NoError(t, e.Flush(ctx), "the carried part commits with the next flush once the claim is back") + require.NoError(t, e.Flush(ctx), "the head flushes once the claim is back") assert.EqualValues(t, 3, k.loadIndex(t, inner).Generation.Term) - assert.Len(t, k.loadIndex(t, inner).Entries, 4) + assert.Len(t, k.loadIndex(t, inner).Entries, 3) // Held while it writes its output, so the tenure ends and restarts before it reaches the commit. gate := faultbackend.NewGate() @@ -156,6 +159,6 @@ func lapsedClaimCommitsNothing(t *testing.T, k Kind) { term.Store(4) require.ErrorIs(t, commit(), bucketindex.ErrSuperseded, "a merge that outlived its tenure commits nothing") - assert.Len(t, k.loadIndex(t, inner).Entries, 4) + assert.Len(t, k.loadIndex(t, inner).Entries, 3) assert.Equal(t, []Row{api(100, 1), api(200, 2), api(300, 3), api(400, 4)}, sortedRows(t, e)) } diff --git a/recordengine/engine.go b/recordengine/engine.go index dc374f7e..5e125fe7 100644 --- a/recordengine/engine.go +++ b/recordengine/engine.go @@ -1407,8 +1407,14 @@ func (e *Engine) flush(ctx context.Context) (rows int, written int64, err error) return 0, 0, nil } - // Checked after the detach so an empty head flushes as a no-op even while fenced. - if err := e.fenceLocked(); err != nil { + // Checked after the detach so an empty head flushes as a no-op even while fenced, and before any + // I/O so a writer without its tenure writes no part it could not commit. + err = e.fenceLocked() + if err == nil { + err = e.tenureLocked(e.term()) + } + + if err != nil { e.head.reattach(detached, detachedBytes) e.mu.Unlock() diff --git a/recordengine/merge.go b/recordengine/merge.go index 5fef5fff..eafeb460 100644 --- a/recordengine/merge.go +++ b/recordengine/merge.go @@ -11,6 +11,7 @@ import ( "go.opentelemetry.io/otel/trace" "go.uber.org/zap" + "github.com/oteldb/storage/backend/bucketindex" "github.com/oteldb/storage/internal/mergestream" "github.com/oteldb/storage/internal/timebucket" ) @@ -673,7 +674,9 @@ const mergeRunFraction = 4 // Close flushes any buffered records to a part and closes the WAL. It does not stop a background // loop — the owner ([storage.Storage]) does that before calling Close. func (e *Engine) Close(ctx context.Context) error { - if _, _, err := e.flush(ctx); err != nil { + // A clustered engine without the shard's claim leaves its head to the WAL, for the next start + // to replay, rather than failing to close. + if _, _, err := e.flush(ctx); err != nil && !errors.Is(err, bucketindex.ErrSuperseded) { return err } diff --git a/storage.go b/storage.go index 3027baaf..ba573a4d 100644 --- a/storage.go +++ b/storage.go @@ -285,6 +285,13 @@ func (s *Storage) Close(ctx context.Context) error { // Final flush: drain every engine's head to a durable part. var firstErr error + // An owner drains while it still holds its claims: once it has left the cluster no commit of its + // is any tenure's, and the engine closes below would leave every head to the WAL, to be flushed + // again by whichever node replays or re-replicates it. + if s.cluster != nil && !s.opts.ReadOnly { + s.flushOwned(ctx) + } + // Leave the cluster first (revoke lease, stop the replica server) so peers stop routing here. if s.cluster != nil { if err := s.cluster.close(ctx); err != nil { @@ -910,12 +917,23 @@ func (f seedFetcher) Unwrap() fetch.Fetcher { return f.inner } // engineCloser is the Close surface both engine types share, so [Storage.Close] drains them all // through one loop. type engineCloser interface { + Flush(ctx context.Context) error Close(ctx context.Context) error CloseWAL() error } // closeEngines drains every tenant engine's head to a durable part and closes its WAL, returning // the first error while still closing the rest. +// flushOwned flushes every engine whose shard this node still holds the claim on. An engine it does +// not own refuses the flush before writing anything, so it is skipped without an error. +func (s *Storage) flushOwned(ctx context.Context) { + for _, eng := range s.allEngines() { + if err := eng.Flush(ctx); err != nil && !errors.Is(err, bucketindex.ErrSuperseded) { + s.obs.Logger(ctx).Warn("flush before leaving the cluster failed", zap.Error(err)) + } + } +} + func (s *Storage) closeEngines(ctx context.Context) error { var firstErr error From 34d72e4d7e69ddf979e682d4d50e8817cf04d683 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Mon, 28 Sep 2026 22:48:38 +0300 Subject: [PATCH 4/8] fix(storage): report a failed owned flush from a clustered Close The flush before leaving the cluster is the only one the node may make, so its failure is Close's error rather than a log line over acked rows left only in the WAL. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KcLq1G8KpWBvM53JaQb1tS --- cluster_close_test.go | 37 +++++++++++++++++++++++++++++++++++++ storage.go | 24 +++++++++++++++--------- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/cluster_close_test.go b/cluster_close_test.go index 18c11051..97338ffb 100644 --- a/cluster_close_test.go +++ b/cluster_close_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/oteldb/storage/backend/faultbackend" "github.com/oteldb/storage/backend/file" "github.com/oteldb/storage/cluster" "github.com/oteldb/storage/cluster/etcd" @@ -65,3 +66,39 @@ func TestClusterCloseFlushesWhileTheClaimIsHeld(t *testing.T) { require.NoError(t, err) require.NoError(t, s.Close(ctx), "a node holding no claim closes cleanly, its head left to the WAL") } + +// TestClusterCloseReportsAFailedOwnedFlush: the flush before leaving the cluster is the only one this +// node may make. Once the claims are gone the engines' own final flush is refused as superseded, so +// a failure of the owned flush has to reach Close's caller — or it reports a clean shutdown while +// acknowledged rows are only in the WAL. +// +//nolint:paralleltest // owns an embedded etcd; runs serially +func TestClusterCloseReportsAFailedOwnedFlush(t *testing.T) { + endpoint := etcdtest.Start(t) + ctx := context.Background() + + inner, err := file.New(t.TempDir()) + require.NoError(t, err) + + be := faultbackend.Wrap(inner) + + s, err := Open(ctx, Options{}, WithBackend(be), WithWALDir(t.TempDir()), WithFlushInterval(-1), + WithCluster(&cluster.Config{ + Etcd: []string{endpoint}, + Self: etcd.Member{ID: "node-a", Addr: "127.0.0.1:0"}, + RF: 1, + PrivateBackend: true, + })) + require.NoError(t, err) + + _, err = s.WriteMetrics(ctx, gaugeBatch("api", "http.requests", []int64{100, 200}, []float64{1, 2})) + require.NoError(t, err) + + const shard = signal.TenantID("default") + + _, owned := s.ownedTenants(ctx, map[signal.TenantID]struct{}{shard: {}})[shard] + require.True(t, owned) + + be.Add(faultbackend.Rule{Kind: faultbackend.CompareAndSwap, Err: assert.AnError}) + require.ErrorIs(t, s.Close(ctx), assert.AnError, "the owned flush failed, and Close says so") +} diff --git a/storage.go b/storage.go index ba573a4d..f272b94f 100644 --- a/storage.go +++ b/storage.go @@ -289,12 +289,12 @@ func (s *Storage) Close(ctx context.Context) error { // is any tenure's, and the engine closes below would leave every head to the WAL, to be flushed // again by whichever node replays or re-replicates it. if s.cluster != nil && !s.opts.ReadOnly { - s.flushOwned(ctx) + firstErr = s.flushOwned(ctx) } // Leave the cluster first (revoke lease, stop the replica server) so peers stop routing here. if s.cluster != nil { - if err := s.cluster.close(ctx); err != nil { + if err := s.cluster.close(ctx); err != nil && firstErr == nil { firstErr = err } } @@ -922,18 +922,24 @@ type engineCloser interface { CloseWAL() error } -// closeEngines drains every tenant engine's head to a durable part and closes its WAL, returning -// the first error while still closing the rest. -// flushOwned flushes every engine whose shard this node still holds the claim on. An engine it does -// not own refuses the flush before writing anything, so it is skipped without an error. -func (s *Storage) flushOwned(ctx context.Context) { +// flushOwned flushes every engine whose shard this node still holds the claim on, returning the first +// failure while still flushing the rest. An engine it does not own refuses the flush before writing +// anything, so it is skipped without an error. A failure has to reach Close's caller: it is the only +// flush this node may make, since the engines' own final flush runs after the claims are gone. +func (s *Storage) flushOwned(ctx context.Context) error { + var firstErr error + for _, eng := range s.allEngines() { - if err := eng.Flush(ctx); err != nil && !errors.Is(err, bucketindex.ErrSuperseded) { - s.obs.Logger(ctx).Warn("flush before leaving the cluster failed", zap.Error(err)) + if err := eng.Flush(ctx); err != nil && !errors.Is(err, bucketindex.ErrSuperseded) && firstErr == nil { + firstErr = errors.Wrap(err, "flush before leaving the cluster") } } + + return firstErr } +// closeEngines drains every tenant engine's head to a durable part and closes its WAL, returning +// the first error while still closing the rest. func (s *Storage) closeEngines(ctx context.Context) error { var firstErr error From 8599a7704886f74b23a11ae3c8897fcd8b14f135 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Mon, 28 Sep 2026 22:48:39 +0300 Subject: [PATCH 5/8] fix(engine): establish a tenure with a fresh-load commit before it commits anything The tenure check is a local read, so a predecessor's held CAS could land after the claim moved. The new owner's first commit now moves the index version from a fresh load, so the held CAS fails and is refused, or landed first and is the new tenure's starting point instead of being rebased beside its inputs. Over private backends identity stays the only protection. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KcLq1G8KpWBvM53JaQb1tS --- ARCHITECTURE.md | 8 +- backend/ARCH.md | 31 ++++-- cluster/ARCH.md | 15 ++- engine/ARCH.md | 22 +++- engine/engine.go | 10 ++ engine/generation_test.go | 2 +- engine/index.go | 155 ++++++++++++++++++----------- engine/merge.go | 12 +++ internal/enginetest/enginetest.go | 4 +- internal/enginetest/tenurefence.go | 91 ++++++++++++++--- recordengine/engine.go | 9 ++ recordengine/index.go | 155 ++++++++++++++++++----------- recordengine/merge.go | 12 +++ 13 files changed, 375 insertions(+), 151 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 77a612a4..298ed73f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -199,9 +199,11 @@ to fail over to: a read overlapping a want fails. safe (`cluster/ARCH.md`, *Lease fencing*). - **A clustered writer commits only as the shard's current tenure.** Every index commit first re-checks that the writer still holds its claim, under the term the operation began with, and - that the index it builds on was not written by a later term (`bucketindex.CheckTenure`). Part - identity is scoped the same way: a block is a `(term, n)` pair, so two tenures that never saw each - other's index cannot allocate the same identity. What the fence leaves possible is duplicate rows, + that the index it builds on was not written by a later term (`bucketindex.CheckTenure`), and a + tenure's first commit re-establishes the index under its term, so over a shared store no displaced + writer lands a commit after its successor's first. Part identity is scoped the same way: a block is + a `(term, n)` pair, so two tenures that never saw each other's index — every handoff over private + backends — cannot allocate the same identity. What the fence leaves possible is duplicate rows, never a collision (`backend/ARCH.md`, *Part identity*). - **The bucket index commits last, conditionally.** It is what makes a part durably visible — and in `recordengine` it also carries the flush watermark (the WAL replay floor, one slot per writer since diff --git a/backend/ARCH.md b/backend/ARCH.md index c33bb08d..cb1446e4 100644 --- a/backend/ARCH.md +++ b/backend/ARCH.md @@ -226,23 +226,32 @@ a demonstration. round trip on the flush path, and it works with the cluster layer absent. Two owners racing a handoff over one index resolve through that CAS: one commit lands, the loser wraps `ErrConflict`, re-reads, and re-allocates above the winner. - **One identity written by two tenures resolves to the later one** (`Entry.Term`, format v7). The - fence below leaves one way for it to arise: two owners merge the same inputs either side of a - handoff, the earlier one committing while its claim is still provable. Both outputs cover the same - blocks at the same level and hold the same rows, so `Lineage.Subsumes` — and through it - `Supersedes`, `Satisfying`, `Revokes` and `Subsumed` — lets the higher `Term` replace the lower. + **One identity written by two tenures resolves to the later one** (`Entry.Term`, format v7). Over + one shared index the fence below keeps it from arising; over private backends two owners can merge + the same inputs either side of a handoff, and the two outputs meet through `cluster/partsync` and + repair. Both cover the same blocks at the same level and hold the same rows, so `Lineage.Subsumes` + — and through it `Supersedes`, `Satisfying`, `Revokes` and `Subsumed` — lets the higher `Term` + replace the lower. - **A clustered writer commits only as the shard's current tenure** (`CheckTenure`, `ErrSuperseded`). Before each CAS attempt the engine checks the term it holds now against the term its operation began under and the term of the index it builds on. It refuses when it holds no claim (a lapsed or fenced lease reads as term 0), when its tenure ended and restarted mid-operation, and when a rebase shows a later tenure wrote the index — a rebase never adopts a higher term, so a displaced writer never commits under its successor's. A refused commit leaves its output unreferenced, for - the orphan sweep. It is a check before a CAS, not inside one: a commit that passes can still land - after the claim lapses, and what bounds that window is the lease fence margin, since a successor - can only acquire once the lease has expired. What the window admits is duplicate rows — an old - owner's last flush, re-flushed by the new owner from its replica head, or overlapping merges — - never a collision, a stuck want or a false hole. A writer with no cluster has no tenure and is not - fenced. + the orphan sweep. A writer with no cluster has no tenure and is not fenced. + The check reads the claim locally, so on its own it cannot stop a commit that passes it and then + lands after the claim moved. **A tenure's first commit is what closes that window**: the new + owner reloads the index and commits it back under its term before any flush, merge or repair of + the tenure may commit (`engine/ARCH.md`, "A clustered commit is fenced by tenure"). That moves the + index version, so a predecessor's held CAS either landed first — and is the new tenure's starting + point — or fails, rebases into the higher term and is refused. The CAS is the only serialization + point two writers share, so this is also why the guarantee is per index: **over a shared store** + a displaced writer never lands a commit after its successor's first one; **over private backends** + (`cluster.Config.PrivateBackend`) each node commits to its own index, nothing orders a displaced + owner's writes against its successor's, and the protection is identity alone — term-scoped blocks + cannot collide and an identical identity resolves to the later tenure. What that admits is + duplicate rows — an old owner's last flush, re-flushed by the new owner from its replica head, or + overlapping merges — never a collision, a stuck want or a false hole. - **`Entries → Removed | Wanted`** is the invariant the repair path enforces: a part leaves `Entries` only into a tombstone (`Removal`, a deliberate deletion) or into a `Want` (an obligation to fetch it back). Conflating the two would make "am I repaired?" unanswerable, which diff --git a/cluster/ARCH.md b/cluster/ARCH.md index 4dfe49b8..fa7e40db 100644 --- a/cluster/ARCH.md +++ b/cluster/ARCH.md @@ -75,9 +75,18 @@ incarnation's writes outrank the dead one's. The term is also what scopes a tenure's **part identity** and **fences its commits**: blocks are allocated as `(term, n)`, and an engine refuses to commit once `Ownership.Term` no longer reports the term its operation began under, or once the index it builds on carries a higher one -(`backend/ARCH.md`, *Part identity*). A handoff therefore never stalls a shard: the new owner flushes -and merges with the freshest view it has. The cost is duplicate rows, never a collision — an old -owner's last flush that no replica mirrored is re-flushed by the new owner from its replica head. +(`backend/ARCH.md`, *Part identity*). A new owner's first commit re-establishes the index under its +term before anything else of the tenure commits. What that buys depends on the mode: + +| mode | what orders a displaced owner's commits against its successor's | +|---|---| +| shared store | the index CAS: once the successor's first commit lands, a predecessor's held commit fails and is refused, and one that landed earlier is the successor's starting point | +| `PrivateBackend` | nothing: each node commits its own index, so the protection is identity — term-scoped blocks never collide, and one identity written twice resolves to the later term | + +A handoff therefore never stalls a shard: the new owner flushes and merges with the freshest view it +has. The cost is duplicate rows, never a collision — in either mode, an old owner's last flush that +no replica mirrored is re-flushed by the new owner from its replica head, and over private backends +two owners' overlapping merges either side of the handoff both stay. `Storage.Close` therefore flushes every engine it owns *before* leaving the cluster, while the claims still hold; after leaving, the engines' final flushes are refused and their heads stay in the WAL. diff --git a/engine/ARCH.md b/engine/ARCH.md index 743b26ca..8495a14e 100644 --- a/engine/ARCH.md +++ b/engine/ARCH.md @@ -149,10 +149,6 @@ The opens sit on the commit path, which is where the cost is. It is bounded: han across the retry loop, and an entry that cannot be opened — a rival merged it away in between — is left out of the readable set but kept in the index, since only its writer knows whether it is live. -**An adopted part this engine's own parts subsume is retired instead** (`dropSubsumedForeignLocked`): -the same merge a displaced tenure committed just before this one, resolved to the later tenure, or -an input a merge here has since consumed. It is left out of the commit and tombstoned, since -carrying it forward keeps its rows live twice; its objects are the orphan sweep's. ### Block identity is allocated by the commit that publishes the part @@ -303,6 +299,24 @@ refused only at the commit keeps its part as a carried, uncommitted part, like a refused merge rolls back and leaves its output to the sweep. `Close` treats the refusal as nothing to flush: the head stays in the WAL for the next start. +**Nothing of a tenure commits before the tenure is established** (`establishTenureLocked`). Flush, +merge and `MergeWith` (ahead of repair) first make the tenure's own commit: a fresh load of the +index (`loadReplica`: no sweep, lost parts become pending wants), committed back under the new term, +which also publishes those wants and any carried part. Until it lands, `tenureLocked` refuses every +commit of the term, so a flush keeps its rows in the head and the WAL and a merge does nothing; the +next maintenance cycle tries again, bounded per attempt like any commit, and a backend that cannot +take the commit could not take the flush either. A load-time want commit skips an unestablished +tenure and leaves the wants pending. + +It is a load and not a rebase because of what a predecessor may have committed after the claim moved +and before this tenure's first write. A rebase carries this engine's own parts forward, so a merge +the predecessor landed in that window would be kept beside the inputs it consumed; a fresh load +makes it this tenure's starting point instead. Once the commit lands, the index version has moved +and a predecessor's held CAS can only fail, rebase and be refused (`backend/ARCH.md`, "A clustered +writer commits only as the shard's current tenure"). That ordering holds per index: over private +backends the successor's commit orders nothing on the predecessor's disk, and identity is the +protection there. + ## A failed load changes nothing, and fences every commit A load is all-or-nothing. `readIndexLocked` reads the index and opens every part it names into diff --git a/engine/engine.go b/engine/engine.go index 64aaad3f..1a61202a 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -283,6 +283,9 @@ type Engine struct { // and raised to a rival's on rebase for the same reason: identity must stay unique over the // shard's whole life, and the live set it could otherwise be derived from shrinks. allocated bucketindex.Block + // established is the ownership term whose first commit has landed: until it equals the current + // term, no commit of that tenure is allowed. See [Engine.establishTenureLocked]. + established uint64 // pendingHoles are the losses the next commit must acknowledge. Like pendingWants they are // held rather than applied on the spot, so a commit that never lands leaves the want // outstanding instead of half-discharged. @@ -1593,6 +1596,13 @@ func (e *Engine) flush(ctx context.Context) (rows int, written int64, err error) // Plan (under lock): detach the head's sample buffers, keeping them readable via e.flushing so a // concurrent fetch never loses them. e.mu.Lock() + + if err := e.establishTenureLocked(ctx); err != nil { + e.mu.Unlock() + + return 0, 0, err + } + detached, detachedBytes, detachedSince := e.head.detach() if detached == nil { e.mu.Unlock() diff --git a/engine/generation_test.go b/engine/generation_test.go index 41d9b4c2..54b3e2f5 100644 --- a/engine/generation_test.go +++ b/engine/generation_test.go @@ -100,7 +100,7 @@ func TestIndexGenerationFollowsTheTerm(t *testing.T) { second := loadIndex(t, be, "default/metrics").Generation assert.EqualValues(t, 9, second.Term) - assert.EqualValues(t, 1, second.Counter, "a new tenure starts its own sequence") + assert.EqualValues(t, 2, second.Counter, "a new tenure starts its own sequence: its establishing commit, then the flush") assert.Equal(t, 1, second.Compare(first)) } diff --git a/engine/index.go b/engine/index.go index c925e7cb..a5835125 100644 --- a/engine/index.go +++ b/engine/index.go @@ -70,33 +70,9 @@ func (e *Engine) commitIndexLocked(ctx context.Context, stamp uint64, guard func return err } - ix := e.nextIndexLocked(ctx) - - version, err := ix.Save(ctx, e.cfg.Backend, e.indexKey(), e.indexVersion) - if err == nil { - e.indexVersion = version - // Only a commit that landed discharges a want: the obligation is dropped from the - // engine's own list here, never while building an index that may not be written. - e.wants = ix.Wanted - e.pendingWants, e.adoptedWants = nil, nil - // Same rule for the blocks this attempt allocated: a part numbered before its CAS - // landed would hold a block the winner took, and the retry would not re-allocate. - for i := range e.pendingBlocks { - a := &e.pendingBlocks[i] - a.part.blocks, a.part.claim, a.part.level, a.part.term = a.blocks, a.claim, a.level, a.term - a.part.pending = nil - } - - e.pendingBlocks = nil - e.holes, e.lostParts = ix.Holes(), ix.LostParts - e.allocated = ix.AllocatedBlocks - e.pendingHoles = nil - - return nil - } - - if !errors.Is(err, bucketindex.ErrConflict) { - return errors.Wrap(err, "save bucket index") + landed, err := e.commitOnceLocked(ctx) + if err != nil || landed { + return err } if err := e.adoptIndexLocked(ctx); err != nil { @@ -114,6 +90,41 @@ func (e *Engine) commitIndexLocked(ctx context.Context, stamp uint64, guard func "commit bucket index after %d attempts", indexCommitAttempts) } +// commitOnceLocked makes one conditional write of the index this engine would commit now, reporting +// whether it landed; a lost race is not an error. Caller holds e.mu. +func (e *Engine) commitOnceLocked(ctx context.Context) (bool, error) { + ix := e.nextIndexLocked(ctx) + + version, err := ix.Save(ctx, e.cfg.Backend, e.indexKey(), e.indexVersion) + if err != nil { + if errors.Is(err, bucketindex.ErrConflict) { + return false, nil + } + + return false, errors.Wrap(err, "save bucket index") + } + + e.indexVersion = version + // Only a commit that landed discharges a want: the obligation is dropped from the engine's own + // list here, never while building an index that may not be written. + e.wants = ix.Wanted + e.pendingWants, e.adoptedWants = nil, nil + // Same rule for the blocks this attempt allocated: a part numbered before its CAS landed would + // hold a block the winner took, and the retry would not re-allocate. + for i := range e.pendingBlocks { + a := &e.pendingBlocks[i] + a.part.blocks, a.part.claim, a.part.level, a.part.term = a.blocks, a.claim, a.level, a.term + a.part.pending = nil + } + + e.pendingBlocks = nil + e.holes, e.lostParts = ix.Holes(), ix.LostParts + e.allocated = ix.AllocatedBlocks + e.pendingHoles = nil + + return true, nil +} + // adoptIndexLocked rebases this engine on the index a rival writer committed: it takes that // index's version (so the retry conditions on it), raises the generation above the rival's (so // the retry supersedes rather than reads as stale), and records the entries the rival added as @@ -267,7 +278,6 @@ func (e *Engine) nextIndexLocked(ctx context.Context) *bucketindex.Index { var assigned []blockAssignment - own := make([]bucketindex.Entry, 0, len(e.parts)) live := make(map[string]struct{}, len(e.parts)) for _, p := range e.parts { @@ -279,17 +289,13 @@ func (e *Engine) nextIndexLocked(ctx context.Context) *bucketindex.Index { assigned = append(assigned, blockAssignment{part: p, blocks: blocks, claim: claim, level: level, term: term}) } - ent := bucketindex.Entry{ + ix.Add(bucketindex.Entry{ Prefix: p.prefix, MinTime: p.minTime, MaxTime: p.maxTime, Blocks: blocks, Claim: claim, Level: level, Term: term, - } - ix.Add(ent) - own = append(own, ent) + }) live[p.prefix] = struct{}{} } - e.dropSubsumedForeignLocked(ix, own) - // The last block handed out, over every source this attempt considered — so the mark only ever // rises, and a shard whose live set has since emptied never renumbers over a part it once held. ix.AllocatedBlocks = alloc.Mark() @@ -756,7 +762,7 @@ func (e *Engine) loadPartsLocked(ctx context.Context, mode loadMode) error { // before the rest of the load so the obligation is durable even if identity recovery fails. A // carried part's identity would be stamped by this commit, which does not hold flushMu, so the // wants then stay pending for the next flush or merge to commit. So do they on a clustered engine - // without the shard's claim, which may not commit at all: the load is then an unclaimed one. + // without an established tenure, which may not commit yet: the load is then an unclaimed one. if len(l.lost) > 0 && mode == loadOwner && !l.carried && e.tenureLocked(e.term()) == nil { if err := e.updateIndexLocked(ctx); err != nil { return errors.Wrap(err, "record repair wants") @@ -891,38 +897,75 @@ func colsTimeRange(cols *flushColumns) (minTime, maxTime int64) { // tenureLocked is the commit fence of a clustered writer ([bucketindex.CheckTenure]) for an // operation that began under term stamp, checked against the term of the index the commit builds on. -// A writer with no cluster has no tenure to lose. Caller holds e.mu. +// It also refuses a tenure not yet established ([Engine.establishTenureLocked]). A writer with no +// cluster has no tenure to lose. Caller holds e.mu. func (e *Engine) tenureLocked(stamp uint64) error { if e.cfg.Term == nil { return nil } - return bucketindex.CheckTenure(stamp, e.term(), e.generation.Term) -} + current := e.term() + if err := bucketindex.CheckTenure(stamp, current, e.generation.Term); err != nil { + return err + } -// dropSubsumedForeignLocked takes out of ix, and tombstones, the adopted entries whose every row this -// engine's own entries hold — the same merge a displaced tenure committed before this one, resolved -// to the later tenure, or a part a merge here has since consumed. Carrying one forward would keep a -// set of rows live twice. Caller holds e.mu. -func (e *Engine) dropSubsumedForeignLocked(ix *bucketindex.Index, own []bucketindex.Entry) { - if len(e.foreign) == 0 { - return + if e.established != current { + return errors.Wrapf(bucketindex.ErrSuperseded, "term %d is not established", current) } - gone := bucketindex.Subsumed(e.foreign, own) - if len(gone) == 0 { - return + return nil +} + +// establishTenure is [Engine.establishTenureLocked] for a caller holding neither lock. +func (e *Engine) establishTenure(ctx context.Context) error { + e.flushMu.Lock() + defer e.flushMu.Unlock() + + e.mu.Lock() + defer e.mu.Unlock() + + return e.establishTenureLocked(ctx) +} + +// establishTenureLocked makes the first commit of the engine's current tenure, before any flush, +// merge or repair of that tenure may commit: a fresh load of the index, committed back under the new +// term. The commit is what closes the window between a predecessor's tenure check and its CAS — the +// index version moves, so a predecessor's held commit can only fail, rebase into this term and be +// refused — and the fresh load is what makes a predecessor commit that landed first this tenure's +// starting point, rather than something a rebase of the view held as a replica would carry the +// consumed inputs back in beside. A no-op once established, without a cluster, or without a claim. +// It is bounded like any commit, and until it lands every commit of the tenure is refused, so a +// flush keeps its rows in the head and the WAL. Caller holds e.flushMu and e.mu. +func (e *Engine) establishTenureLocked(ctx context.Context) error { + if e.cfg.Term == nil || e.cfg.Backend == nil { + return nil } - for prefix := range gone { - ix.Remove(prefix) - delete(e.foreignParts, prefix) - e.removals = append(e.removals, bucketindex.Removal{Prefix: prefix, Generation: e.generation}) + term := e.term() + if term == 0 || e.established == term { + return nil } - e.foreign = slices.DeleteFunc(e.foreign, func(ent bucketindex.Entry) bool { - _, ok := gone[ent.Prefix] + for range indexCommitAttempts { + if err := e.loadPartsLocked(ctx, loadReplica); err != nil { + return errors.Wrap(err, "load bucket index to establish the tenure") + } + + if err := bucketindex.CheckTenure(term, e.term(), e.generation.Term); err != nil { + return err + } + + landed, err := e.commitOnceLocked(ctx) + if err != nil { + return err + } + + if landed { + e.established = term + + return nil + } + } - return ok - }) + return errors.Wrapf(bucketindex.ErrConflict, "establish term %d after %d attempts", term, indexCommitAttempts) } diff --git a/engine/merge.go b/engine/merge.go index aeef7e09..bacab38e 100644 --- a/engine/merge.go +++ b/engine/merge.go @@ -62,6 +62,11 @@ func (e *Engine) MergeWith(ctx context.Context, opts MergeOptions) error { // Repair first: a part pulled back from a peer joins this cycle's compaction, and a merge that // cannot repair still compacts. + // Repair commits under the tenure too, so it has to be established before repair runs. + if err := e.establishTenure(ctx); err != nil { + return err + } + e.repairWants(ctx) res, err := e.merge(ctx, opts) @@ -107,6 +112,13 @@ func (e *Engine) merge(ctx context.Context, opts MergeOptions) (mergeResult, err // Plan (under lock): snapshot the source parts (immutable backing). Output part ids are minted one // at a time, as the parts are written. e.mu.Lock() + + if err := e.establishTenureLocked(ctx); err != nil { + e.mu.Unlock() + + return mergeResult{}, err + } + src := e.parts adopted := slices.Collect(maps.Values(e.foreignParts)) e.mu.Unlock() diff --git a/internal/enginetest/enginetest.go b/internal/enginetest/enginetest.go index 8d7a5cc3..da82dfe2 100644 --- a/internal/enginetest/enginetest.go +++ b/internal/enginetest/enginetest.go @@ -291,7 +291,9 @@ var suite = []struct { {"SplitBrainMergeSameInputs", splitBrainMergeSameInputs}, {"SplitBrainMergeOverlappingInputs", splitBrainMergeOverlappingInputs}, {"DisplacedWriterKeepsItsTerm", displacedWriterKeepsItsTerm}, - {"SplitBrainMergeResolvesToTheLaterTenure", splitBrainMergeResolvesToTheLaterTenure}, + {"StaleCommitCannotLandAfterTheSuccessorEstablishes", staleCommitCannotLandAfterTheSuccessorEstablishes}, + {"PredecessorCommitLandsBeforeTheSuccessorEstablishes", predecessorCommitLandsBeforeTheSuccessorEstablishes}, + {"UnestablishedTenureKeepsRowsInTheHead", unestablishedTenureKeepsRowsInTheHead}, {"TenuresAllocateDisjointBlocks", tenuresAllocateDisjointBlocks}, {"LapsedClaimCommitsNothing", lapsedClaimCommitsNothing}, {"WritingTermSurvivesTheNextTenure", writingTermSurvivesTheNextTenure}, diff --git a/internal/enginetest/tenurefence.go b/internal/enginetest/tenurefence.go index 60013857..53477e3f 100644 --- a/internal/enginetest/tenurefence.go +++ b/internal/enginetest/tenurefence.go @@ -14,33 +14,64 @@ import ( "github.com/oteldb/storage/backend/faultbackend" ) -// splitBrainMergeResolvesToTheLaterTenure is the other order of splitBrainMergeSameInputs: the -// displaced tenure A still holds its claim when it commits its merge, and B's merge of the same two -// parts lands second. B's rebase adopts A's output, finds its own output to be the same identity -// written by a later tenure, and retires A's rather than keeping both. -func splitBrainMergeResolvesToTheLaterTenure(t *testing.T, k Kind) { +// staleCommitCannotLandAfterTheSuccessorEstablishes is the check-to-CAS window: A's merge passes its +// tenure check and is held inside the index CAS while the claim moves to B. B's first write is the +// commit that establishes its tenure, which changes the index's version, so A's held CAS can only +// fail, rebase into B's term, and be refused — whatever A's local view of its claim still says. +func staleCommitCannotLandAfterTheSuccessorEstablishes(t *testing.T, k Kind) { t.Helper() ctx := context.Background() inner := backend.Memory() - bBe := faultbackend.Wrap(inner) - want := []Row{api(100, 1), api(200, 2)} + aBe := faultbackend.Wrap(inner) + want := []Row{api(100, 1), api(200, 2), api(300, 3)} - a := k.openTenure(t, inner, displacedTerm, 0) - k.flushEach(t, a, inner, want...) + a := k.openTenure(t, aBe, displacedTerm, 0) + k.flushEach(t, a, inner, want[:2]...) - b := k.openTenure(t, bBe, ownerTerm, 0) - commitB := stallMerge(t, b, gateIndexCommit(bBe)) + commitA := stallMerge(t, a, gateIndexCommit(aBe)) - require.NoError(t, a.ForceMerge(ctx)) - require.NoError(t, commitB()) + b := k.openTenure(t, inner, ownerTerm, 0) + b.Append(t, want[2]) + require.NoError(t, b.Flush(ctx)) + require.EqualValues(t, ownerTerm, k.loadIndex(t, inner).Generation.Term) + require.ErrorIs(t, commitA(), bucketindex.ErrSuperseded, "the held commit lands against a moved index") + + ix := k.loadIndex(t, inner) + assert.Len(t, ix.Entries, 3, "A's output never became live") k.requireStoredOnce(t, inner, want) +} + +// predecessorCommitLandsBeforeTheSuccessorEstablishes is the other side of the same window: A's held +// merge lands after B took the claim but before B wrote anything. B establishes its tenure over a +// fresh load of the index, not a rebase of the view it held as a replica, so A's merge is B's starting +// point — the inputs A consumed are not carried back in beside A's output. +func predecessorCommitLandsBeforeTheSuccessorEstablishes(t *testing.T, k Kind) { + t.Helper() + + ctx := context.Background() + inner := backend.Memory() + aBe := faultbackend.Wrap(inner) + want := []Row{api(100, 1), api(200, 2), api(300, 3)} + + a := k.openTenure(t, aBe, displacedTerm, 0) + k.flushEach(t, a, inner, want[:2]...) + + commitA := stallMerge(t, a, gateIndexCommit(aBe)) + + b := k.openTenure(t, inner, ownerTerm, 0) + require.Equal(t, 2, b.PartCount(), "B's view is the two inputs") + + require.NoError(t, commitA(), "A's merge lands before any write of B's tenure") + + b.Append(t, want[2]) + require.NoError(t, b.Flush(ctx)) ix := k.loadIndex(t, inner) - require.Len(t, ix.Entries, 1) - assert.EqualValues(t, ownerTerm, ix.Entries[0].Term, "the later tenure's output is the one kept") - assert.Len(t, ix.Removed, 3, "both inputs and the earlier output are tombstoned, not dropped silently") + assert.EqualValues(t, ownerTerm, ix.Generation.Term) + assert.Len(t, ix.Entries, 2, "A's output and B's flush, and not A's inputs again") + k.requireStoredOnce(t, inner, want) } // tenuresAllocateDisjointBlocks is #725's block collision: two tenures whose indexes diverged — a @@ -86,6 +117,34 @@ func tenuresAllocateDisjointBlocks(t *testing.T, k Kind) { assert.False(t, merged[0].Blocks.Contains(pa.Blocks), "the successor never merged the displaced part") } +// unestablishedTenureKeepsRowsInTheHead: until a new tenure's first commit lands, nothing of it +// commits. A flush whose establishing commit cannot land fails without writing a part, keeps its rows +// in the head, and the next flush establishes the tenure and publishes them. +func unestablishedTenureKeepsRowsInTheHead(t *testing.T, k Kind) { + t.Helper() + + ctx := context.Background() + inner := backend.Memory() + be := faultbackend.Wrap(inner) + + e := k.openTenure(t, be, ownerTerm, 0) + e.Append(t, api(100, 1)) + + be.Add(faultbackend.Rule{Kind: faultbackend.CompareAndSwap, Err: assert.AnError}) + require.ErrorIs(t, e.Flush(ctx), assert.AnError) + assert.Empty(t, k.partDirs(ctx, t, inner), "no part written for a tenure that could not establish") + assert.Equal(t, 1, e.HeadRows()) + + be.Reset() + require.NoError(t, e.Flush(ctx)) + + ix := k.loadIndex(t, inner) + require.Len(t, ix.Entries, 1) + assert.Equal(t, bucketindex.Generation{Term: ownerTerm, Counter: 2}, ix.Generation, + "the establishing commit, then the flush") + assert.Equal(t, []Row{api(100, 1)}, sortedRows(t, e)) +} + // writingTermSurvivesTheNextTenure pins that a part's writing term is part of its identity, not of // the engine that holds it: a later tenure loading the index and committing its own part carries // every earlier entry's term through unchanged. diff --git a/recordengine/engine.go b/recordengine/engine.go index 5e125fe7..712306dc 100644 --- a/recordengine/engine.go +++ b/recordengine/engine.go @@ -259,6 +259,9 @@ type Engine struct { // and raised to a rival's on rebase for the same reason: identity must stay unique over the // shard's whole life, and the live set it could otherwise be derived from shrinks. allocated bucketindex.Block + // established is the ownership term whose first commit has landed: until it equals the current + // term, no commit of that tenure is allowed. See [Engine.establishTenureLocked]. + established uint64 // pendingHoles are the losses the next commit must acknowledge. Like pendingWants they are // held rather than applied on the spot, so a commit that never lands leaves the want // outstanding instead of half-discharged. @@ -1399,6 +1402,12 @@ func (e *Engine) flush(ctx context.Context) (rows int, written int64, err error) return 0, 0, err } + if err := e.establishTenureLocked(ctx); err != nil { + e.mu.Unlock() + + return 0, 0, err + } + detached, detachedBytes := e.head.detach() if detached == nil { e.mu.Unlock() diff --git a/recordengine/index.go b/recordengine/index.go index dbe412c9..325e9f6a 100644 --- a/recordengine/index.go +++ b/recordengine/index.go @@ -61,33 +61,9 @@ func (e *Engine) commitIndexLocked(ctx context.Context, stamp uint64) error { return err } - ix := e.nextIndexLocked(ctx) - - version, err := ix.Save(ctx, e.cfg.Backend, e.indexKey(), e.indexVersion) - if err == nil { - e.indexVersion = version - // Only a commit that landed discharges a want: the obligation is dropped from the - // engine's own list here, never while building an index that may not be written. - e.wants = ix.Wanted - e.pendingWants, e.adoptedWants = nil, nil - // Same rule for the blocks this attempt allocated: a part numbered before its CAS - // landed would hold a block the winner took, and the retry would not re-allocate. - for i := range e.pendingBlocks { - a := &e.pendingBlocks[i] - a.part.blocks, a.part.claim, a.part.level, a.part.term = a.blocks, a.claim, a.level, a.term - a.part.pending = nil - } - - e.pendingBlocks = nil - e.holes, e.lostParts = ix.Holes(), ix.LostParts - e.allocated = ix.AllocatedBlocks - e.pendingHoles = nil - - return nil - } - - if !errors.Is(err, bucketindex.ErrConflict) { - return &commitUnknownError{err: errors.Wrap(err, "save bucket index")} + landed, err := e.commitOnceLocked(ctx) + if err != nil || landed { + return err } if err := e.adoptIndexLocked(ctx); err != nil { @@ -99,6 +75,41 @@ func (e *Engine) commitIndexLocked(ctx context.Context, stamp uint64) error { "commit bucket index after %d attempts", indexCommitAttempts) } +// commitOnceLocked makes one conditional write of the index this engine would commit now, reporting +// whether it landed; a lost race is not an error. Caller holds e.mu. +func (e *Engine) commitOnceLocked(ctx context.Context) (bool, error) { + ix := e.nextIndexLocked(ctx) + + version, err := ix.Save(ctx, e.cfg.Backend, e.indexKey(), e.indexVersion) + if err != nil { + if errors.Is(err, bucketindex.ErrConflict) { + return false, nil + } + + return false, &commitUnknownError{err: errors.Wrap(err, "save bucket index")} + } + + e.indexVersion = version + // Only a commit that landed discharges a want: the obligation is dropped from the engine's own + // list here, never while building an index that may not be written. + e.wants = ix.Wanted + e.pendingWants, e.adoptedWants = nil, nil + // Same rule for the blocks this attempt allocated: a part numbered before its CAS landed would + // hold a block the winner took, and the retry would not re-allocate. + for i := range e.pendingBlocks { + a := &e.pendingBlocks[i] + a.part.blocks, a.part.claim, a.part.level, a.part.term = a.blocks, a.claim, a.level, a.term + a.part.pending = nil + } + + e.pendingBlocks = nil + e.holes, e.lostParts = ix.Holes(), ix.LostParts + e.allocated = ix.AllocatedBlocks + e.pendingHoles = nil + + return true, nil +} + // commitUnknownError is a bucket-index save that failed without saying whether it landed. Every // other [Engine.updateIndexLocked] failure is a commit that definitely did not. type commitUnknownError struct{ err error } @@ -267,7 +278,6 @@ func (e *Engine) nextIndexLocked(ctx context.Context) *bucketindex.Index { var assigned []blockAssignment - own := make([]bucketindex.Entry, 0, len(e.parts)) live := make(map[string]struct{}, len(e.parts)) for _, p := range e.parts { @@ -279,17 +289,13 @@ func (e *Engine) nextIndexLocked(ctx context.Context) *bucketindex.Index { assigned = append(assigned, blockAssignment{part: p, blocks: blocks, claim: claim, level: level, term: term}) } - ent := bucketindex.Entry{ + ix.Add(bucketindex.Entry{ Prefix: p.prefix, MinTime: p.minTime, MaxTime: p.maxTime, Blocks: blocks, Claim: claim, Level: level, Term: term, - } - ix.Add(ent) - own = append(own, ent) + }) live[p.prefix] = struct{}{} } - e.dropSubsumedForeignLocked(ix, own) - // The last block handed out, over every source this attempt considered — so the mark only ever // rises, and a shard whose live set has since emptied never renumbers over a part it once held. ix.AllocatedBlocks = alloc.Mark() @@ -745,7 +751,7 @@ func (e *Engine) loadPartsLocked(ctx context.Context, mode loadMode) error { // before the rest of the load so the obligation is durable even if identity recovery fails. A // carried part's identity would be stamped by this commit, which does not hold flushMu, so the // wants then stay pending for the next flush or merge to commit. So do they on a clustered engine - // without the shard's claim, which may not commit at all: the load is then an unclaimed one. + // without an established tenure, which may not commit yet: the load is then an unclaimed one. if len(l.lost) > 0 && mode == loadOwner && !l.carried && e.tenureLocked(e.term()) == nil { if err := e.updateIndexLocked(ctx); err != nil { return errors.Wrap(err, "record repair wants") @@ -905,38 +911,75 @@ func decodeSeriesSet(data []byte, fn func(signal.Series)) error { // tenureLocked is the commit fence of a clustered writer ([bucketindex.CheckTenure]) for an // operation that began under term stamp, checked against the term of the index the commit builds on. -// A writer with no cluster has no tenure to lose. Caller holds e.mu. +// It also refuses a tenure not yet established ([Engine.establishTenureLocked]). A writer with no +// cluster has no tenure to lose. Caller holds e.mu. func (e *Engine) tenureLocked(stamp uint64) error { if e.cfg.Term == nil { return nil } - return bucketindex.CheckTenure(stamp, e.term(), e.generation.Term) -} + current := e.term() + if err := bucketindex.CheckTenure(stamp, current, e.generation.Term); err != nil { + return err + } -// dropSubsumedForeignLocked takes out of ix, and tombstones, the adopted entries whose every row this -// engine's own entries hold — the same merge a displaced tenure committed before this one, resolved -// to the later tenure, or a part a merge here has since consumed. Carrying one forward would keep a -// set of rows live twice. Caller holds e.mu. -func (e *Engine) dropSubsumedForeignLocked(ix *bucketindex.Index, own []bucketindex.Entry) { - if len(e.foreign) == 0 { - return + if e.established != current { + return errors.Wrapf(bucketindex.ErrSuperseded, "term %d is not established", current) } - gone := bucketindex.Subsumed(e.foreign, own) - if len(gone) == 0 { - return + return nil +} + +// establishTenure is [Engine.establishTenureLocked] for a caller holding neither lock. +func (e *Engine) establishTenure(ctx context.Context) error { + e.flushMu.Lock() + defer e.flushMu.Unlock() + + e.mu.Lock() + defer e.mu.Unlock() + + return e.establishTenureLocked(ctx) +} + +// establishTenureLocked makes the first commit of the engine's current tenure, before any flush, +// merge or repair of that tenure may commit: a fresh load of the index, committed back under the new +// term. The commit is what closes the window between a predecessor's tenure check and its CAS — the +// index version moves, so a predecessor's held commit can only fail, rebase into this term and be +// refused — and the fresh load is what makes a predecessor commit that landed first this tenure's +// starting point, rather than something a rebase of the view held as a replica would carry the +// consumed inputs back in beside. A no-op once established, without a cluster, or without a claim. +// It is bounded like any commit, and until it lands every commit of the tenure is refused, so a +// flush keeps its rows in the head and the WAL. Caller holds e.flushMu and e.mu. +func (e *Engine) establishTenureLocked(ctx context.Context) error { + if e.cfg.Term == nil || e.cfg.Backend == nil { + return nil } - for prefix := range gone { - ix.Remove(prefix) - delete(e.foreignParts, prefix) - e.removals = append(e.removals, bucketindex.Removal{Prefix: prefix, Generation: e.generation}) + term := e.term() + if term == 0 || e.established == term { + return nil } - e.foreign = slices.DeleteFunc(e.foreign, func(ent bucketindex.Entry) bool { - _, ok := gone[ent.Prefix] + for range indexCommitAttempts { + if err := e.loadPartsLocked(ctx, loadReplica); err != nil { + return errors.Wrap(err, "load bucket index to establish the tenure") + } + + if err := bucketindex.CheckTenure(term, e.term(), e.generation.Term); err != nil { + return err + } + + landed, err := e.commitOnceLocked(ctx) + if err != nil { + return err + } + + if landed { + e.established = term + + return nil + } + } - return ok - }) + return errors.Wrapf(bucketindex.ErrConflict, "establish term %d after %d attempts", term, indexCommitAttempts) } diff --git a/recordengine/merge.go b/recordengine/merge.go index eafeb460..34877d69 100644 --- a/recordengine/merge.go +++ b/recordengine/merge.go @@ -73,6 +73,11 @@ func (e *Engine) MergeWith(ctx context.Context, opts MergeOptions) error { // Repair first: a part pulled back from a peer joins this cycle's compaction, and a merge that // cannot repair still compacts. + // Repair commits under the tenure too, so it has to be established before repair runs. + if err := e.establishTenure(ctx); err != nil { + return err + } + e.repairWants(ctx) res, err := e.merge(ctx, opts) @@ -135,6 +140,13 @@ func (e *Engine) mergeHeld(ctx context.Context, opts MergeOptions, abandoned *[] // Plan (under lock): snapshot the source parts (immutable backing). Output part ids are minted one // at a time, as the parts are written. e.mu.Lock() + + if err := e.establishTenureLocked(ctx); err != nil { + e.mu.Unlock() + + return mergeResult{}, err + } + src := e.parts e.mu.Unlock() From 7d7016b5555305b27ba5c2c77df10d9a4b1236d2 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Mon, 28 Sep 2026 23:16:24 +0300 Subject: [PATCH 6/8] fix(cluster): forget the claims when the lease is revoked on close Ownership went on reporting the revoked tenures, so an engine's final flush after leaving the cluster committed under an ended term. An engine Close now also releases its WAL when the flush failed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KcLq1G8KpWBvM53JaQb1tS --- cluster.go | 4 ++++ cluster/etcd/ownership.go | 10 ++++++++++ cluster_close_test.go | 1 + engine/merge.go | 14 +++++++++----- recordengine/merge.go | 14 +++++++++----- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/cluster.go b/cluster.go index c7fe3dba..1fc2a84f 100644 --- a/cluster.go +++ b/cluster.go @@ -1517,6 +1517,10 @@ func (n *clusterNode) close(ctx context.Context) error { firstErr = err } + // The lease went with the claims, so no commit after this is any tenure's: an engine's final + // flush past this point is refused and leaves its head in the WAL. + n.ownership.Forget() + if err := n.server.Shutdown(ctx); err != nil && firstErr == nil { firstErr = err } diff --git a/cluster/etcd/ownership.go b/cluster/etcd/ownership.go index 552bfd66..90ef4c84 100644 --- a/cluster/etcd/ownership.go +++ b/cluster/etcd/ownership.go @@ -76,6 +76,16 @@ func (o *Ownership) SetLease(id clientv3.LeaseID) { o.held = make(map[string]uint64) } +// Forget drops every claim this node believes it holds, without touching etcd: for after the lease +// that bound them was revoked, when the keys are already gone and [Ownership.Term] would otherwise +// go on reporting tenures that have ended. +func (o *Ownership) Forget() { + o.mu.Lock() + defer o.mu.Unlock() + + o.held = make(map[string]uint64) +} + // SetFence installs the predicate that reports this node's claims unprovable — wire it to // [Membership.Fenced]. While it reports true this node holds nothing as far as every caller is // concerned: [Ownership.Term] disclaims, [Ownership.Owned] is empty, and [Ownership.Reconcile] diff --git a/cluster_close_test.go b/cluster_close_test.go index 97338ffb..3ee261bf 100644 --- a/cluster_close_test.go +++ b/cluster_close_test.go @@ -53,6 +53,7 @@ func TestClusterCloseFlushesWhileTheClaimIsHeld(t *testing.T) { _, owned := s.ownedTenants(ctx, map[signal.TenantID]struct{}{shard: {}})[shard] require.True(t, owned) require.NoError(t, s.Close(ctx)) + assert.False(t, s.claimsShard(shard), "leaving the cluster ends the tenure") s = open() t.Cleanup(func() { _ = s.Close(ctx) }) diff --git a/engine/merge.go b/engine/merge.go index bacab38e..10d4b32b 100644 --- a/engine/merge.go +++ b/engine/merge.go @@ -809,19 +809,23 @@ func mergeStreamedSeries( // loop — the owner ([storage.Storage]) does that before calling Close. func (e *Engine) Close(ctx context.Context) error { // A clustered engine without the shard's claim leaves its head to the WAL, for the next start - // to replay, rather than failing to close. - if _, _, err := e.flush(ctx); err != nil && !errors.Is(err, bucketindex.ErrSuperseded) { - return err + // to replay, rather than failing to close. A flush that failed otherwise still closes the WAL: + // its segments hold the head for the next start either way. + _, _, flushErr := e.flush(ctx) + if errors.Is(flushErr, bucketindex.ErrSuperseded) { + flushErr = nil } e.mu.Lock() defer e.mu.Unlock() if e.cfg.WAL != nil { - return e.cfg.WAL.Close() + if err := e.cfg.WAL.Close(); err != nil && flushErr == nil { + return err + } } - return nil + return flushErr } // CloseWAL closes the engine's open WAL segment file handle without flushing the head or diff --git a/recordengine/merge.go b/recordengine/merge.go index 34877d69..353dee38 100644 --- a/recordengine/merge.go +++ b/recordengine/merge.go @@ -687,19 +687,23 @@ const mergeRunFraction = 4 // loop — the owner ([storage.Storage]) does that before calling Close. func (e *Engine) Close(ctx context.Context) error { // A clustered engine without the shard's claim leaves its head to the WAL, for the next start - // to replay, rather than failing to close. - if _, _, err := e.flush(ctx); err != nil && !errors.Is(err, bucketindex.ErrSuperseded) { - return err + // to replay, rather than failing to close. A flush that failed otherwise still closes the WAL: + // its segments hold the head for the next start either way. + _, _, flushErr := e.flush(ctx) + if errors.Is(flushErr, bucketindex.ErrSuperseded) { + flushErr = nil } e.mu.Lock() defer e.mu.Unlock() if e.cfg.WAL != nil { - return e.cfg.WAL.Close() + if err := e.cfg.WAL.Close(); err != nil && flushErr == nil { + return err + } } - return nil + return flushErr } // CloseWAL closes the engine's open WAL segment file handle without flushing the head or From 6fcc35ca6589dc89589fc5c164b07c9c9caede66 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Mon, 28 Sep 2026 23:44:15 +0300 Subject: [PATCH 7/8] fix(engine): try tenure establishment once per flush and merge, and expose it A failed establishment now costs one load and one CAS per operation, like a failing flush, instead of a retry loop under the locks. storage.index.establish_failures counts the attempts and SignalStats.TenureUnestablished shows an engine that commits nothing until one lands. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KcLq1G8KpWBvM53JaQb1tS --- ADMIN.md | 2 ++ engine/ARCH.md | 6 ++-- engine/engine.go | 8 ++++- engine/enginetest_test.go | 2 +- engine/index.go | 53 ++++++++++++++++++++---------- inspect.go | 8 +++++ internal/enginetest/enginetest.go | 2 ++ internal/enginetest/tenurefence.go | 50 ++++++++++++++++++++++++---- internal/obs/corruption.go | 31 ++++++++++++++--- recordengine/engine.go | 8 ++++- recordengine/enginetest_test.go | 2 +- recordengine/index.go | 53 ++++++++++++++++++++---------- 12 files changed, 173 insertions(+), 52 deletions(-) diff --git a/ADMIN.md b/ADMIN.md index 02731436..81032d48 100644 --- a/ADMIN.md +++ b/ADMIN.md @@ -100,6 +100,7 @@ taking only a brief per-engine read lock to copy counters — safe to poll at da | `LostParts` | the shard's **monotone** data-loss count as its bucket index records it: every hole its writers have ever committed, revoked ones included. It never decreases and every owner reads the same number, so it is the durable answer to "did this shard ever lose data?" — unlike `Holes`, which is the current state. Alert on any increase | | `IndexFenced` | this engine's last bucket-index load failed — the index was unreadable, or a part it names could not be opened for a reason other than being gone (a corrupt manifest, a backend error, a format this binary cannot read) — so it **refuses every commit**: flush, merge, retention, repair and `CompactNow` fail with an error wrapping `bucketindex.ErrFenced`. A commit from an engine whose view is not the stored index could drop every entry it never opened. Without a cluster, reads keep answering from the last part set that loaded, and this flag with `IndexLoadError` is the health error to alert on; a cluster node instead disclaims every read of the shard (`rpc.shard_incomplete`), so reads fail over to another owner, and fail when every owner is fenced. Flushes keep their records in the head, which therefore grows until `tenant.Limits.MaxInFlightBytes` sheds writes (without bound when it is zero, the default); the WAL keeps them durable meanwhile. Every maintenance cycle retries the load, and the first that succeeds clears this and lets the held-back flush commit. A part found corrupt by three consecutive loads is handed to repair: the next load records it as a want (`WantedParts`, `corruption.detected{disposition="wanted"}`) and the fence lifts. Any other failure in between starts the count over. Two causes never clear on their own: a backend that keeps failing, and a part in a format newer than this binary (`index.fenced_loads{reason="unsupported_version"}`), which stays fenced until the node runs a release that reads it | | `IndexLoadError` | the error of the failed load behind `IndexFenced`, naming the index or part and why (`… corrupt metadata …`, `… unsupported version N …`, or the backend's error); empty while not fenced | +| `TenureUnestablished` | this cluster node holds the shard's claim but its tenure's first index commit has not landed. A new owner reloads the index and commits it back under its term before anything else of the tenure may commit, so a displaced owner's held commit cannot land after it (`engine/ARCH.md`, *A clustered commit is fenced by tenure*). Until it lands the engine commits nothing — like `IndexFenced`, flushes keep their records in the head, which grows until `tenant.Limits.MaxInFlightBytes` sheds writes (without bound when it is zero, the default); the WAL keeps them durable — and every flush and merge makes one more attempt, counted in `index.establish_failures`. Set for a moment after every handoff; one that stays set is an index this node cannot load or commit to | | `HasReadGap` / `ReadGapAfterUnixNano` | this node holds the shard but came back without its unflushed head (a restart, or a rebalance handing it over), so it disclaims reads at or past that timestamp and lets another owner answer. Clears when a flush by the shard's compaction owner puts newer data in this node's parts. `math.MinInt64` means the shard had no parts at all — nothing here is known to be complete | Part *byte* sizes are intentionally omitted from `Inspect` (they would need backend stat calls) — use @@ -254,6 +255,7 @@ Metric instruments (all prefixed `storage.`): | `wal.sync_failures` / `wal.sync_failing` | — | background fsyncs that failed, and a gauge: 1 while the latest sync pass had a failure (`StoreStats.WALSync`). A non-zero gauge means `WALSyncInterval`'s power-loss bound is not holding; the sync retries every tick, so it clears on its own once the disk recovers | | `corruption.detected` | `component`, `disposition` | on-disk artifacts that failed an integrity check. `disposition=fatal` is an operation that failed on one: a corrupt manifest or bucket index (`part`, `bucket_index`) fails the load. These count only errors carrying a corruption sentinel, never a backend that failed to answer. `disposition=wanted` is a part that stayed corrupt over three consecutive loads and was handed to repair as a want (`part`), one count when it is handed over. `disposition=tolerated` is corruption the engine fell back from and served on: a bloom sidecar (`bloom` — the part stops pruning on that column), a part's identity object (`part_identity` — its rows stay unresolvable until a merge rewrites the part), the paged series index or series-stats sidecar (`series_index`, `series_stats` — slower opens and aggregates), marks (`marks` — no granule pruning), an adopted part (`part` — left out of the read set), a record part whose rows are not timestamp-ordered within a stream, or whose stream column is not grouped in stream order (`stream_order` — merged without losing a row from a whole decode, once per merge that reads it, but its windowed reads are already unreliable), a damaged WAL region (`wal` — one count per region replay skipped at `Open`; its records are lost, a Warn log names the segment, offset and bytes, and the segment is copied aside as `{segment}.damaged`, the newest four kept). A tolerated artifact counts at every open or first read that meets it, so a steady rate is one damaged part read repeatedly, not new damage | | `index.fenced_loads` | `signal`, `reason` | bucket-index loads that failed and left the engine refusing commits (`SignalStats.IndexFenced`). `reason` is `corrupt` (a damaged index or part; a part that stays so is handed to repair after three loads), `unsupported_version` (a part written by a newer release; never clears until this node is upgraded) or `unavailable` (the backend failed to answer). A fenced engine retries once per maintenance cycle and each failure counts again, so a steady rate is one engine that stays fenced; a single count that stops is a transient failure the next retry cleared. A corrupt artifact behind it also counts in `corruption.detected` with `disposition=fatal` | +| `index.establish_failures` | `signal`, `reason` | attempts to make a new tenure's first index commit that did not land (`SignalStats.TenureUnestablished`). `reason` is `conflict` (the commit lost the index CAS to another writer — a predecessor still committing, or another node that believes it owns the shard) or `unavailable` (the backend failed the load or the commit). Each flush and each merge makes one attempt, so a steady rate is one engine whose tenure never establishes and whose head is growing; a few counts right after a handoff are the predecessor's last commits | | `parts.orphans_swept` | `signal` | part objects no index entry names, deleted by the sweep at open: what failed flushes and merges of the previous process left behind. It moves only at `Open` (and a cluster owner's backfill reload) — a flush that fails at runtime leaves its objects until the next restart — so a flat line says nothing about orphans accumulating now | | `tenant.reserved_skipped` | `source` | a tenant whose id is reserved (`wal`, any case, or containing `\`) was found and left untouched, each also logged at Error with the tenant and prefix. `source` is `recovery` (a bucket index in the backend), `wal_replay` (a WAL directory), `bootstrap` (an etcd shard claim) or `partsync` (a peer's prefix). Only a release predating the reservation writes such a tenant; its data stays on disk, unserved, until an operator moves it | | `parts.orphans_deferred` | `signal` | unnamed part objects the same sweep **kept** because their part id is younger than `Options.OrphanGrace`: over a shared store such a part may be another node's flush or merge that has not committed yet. Counted by every load that defers them, so it is a rate of deferrals rather than a count of distinct objects. A steady rate on a store only this node writes means failed flushes whose leftovers wait for a load after they age; it only matters if it keeps growing, which is disk or inodes held by debris | diff --git a/engine/ARCH.md b/engine/ARCH.md index 8495a14e..66d6e9db 100644 --- a/engine/ARCH.md +++ b/engine/ARCH.md @@ -304,8 +304,10 @@ merge and `MergeWith` (ahead of repair) first make the tenure's own commit: a fr index (`loadReplica`: no sweep, lost parts become pending wants), committed back under the new term, which also publishes those wants and any carried part. Until it lands, `tenureLocked` refuses every commit of the term, so a flush keeps its rows in the head and the WAL and a merge does nothing; the -next maintenance cycle tries again, bounded per attempt like any commit, and a backend that cannot -take the commit could not take the flush either. A load-time want commit skips an unestablished +next flush or merge tries again — one load and one CAS each, never a loop holding the locks. An +unestablished tenure therefore drains nothing, exactly like a flush that keeps failing: the head and +the WAL hold the rows and grow, and the only bound on that is the optional per-tenant in-flight limit +(mandatory lossless backpressure is #146). A load-time want commit skips an unestablished tenure and leaves the wants pending. It is a load and not a rebase because of what a predecessor may have committed after the claim moved diff --git a/engine/engine.go b/engine/engine.go index 1a61202a..a84c9437 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -572,6 +572,10 @@ type Stats struct { IndexFenced bool // IndexLoadErr is the error of the failed load behind IndexFenced, nil while not fenced. IndexLoadErr error + // TenureUnestablished is set while this clustered engine holds the shard's claim but its + // tenure's first commit has not landed: it commits nothing, so flushes keep their rows in the + // head, and every flush and merge tries again. + TenureUnestablished bool } // Stats returns an in-memory snapshot of the engine's state under a single read lock. It does no @@ -593,7 +597,9 @@ func (e *Engine) Stats() Stats { LostParts: e.lostParts, IndexFenced: e.loadErr != nil, IndexLoadErr: e.loadErr, - MaxTime: e.head.newest, + + TenureUnestablished: e.tenureUnestablishedLocked(), + MaxTime: e.head.newest, } for _, buf := range e.head.samples { diff --git a/engine/enginetest_test.go b/engine/enginetest_test.go index 4c0ff84c..e43556cf 100644 --- a/engine/enginetest_test.go +++ b/engine/enginetest_test.go @@ -94,7 +94,7 @@ func (e metricEngine) Stats() enginetest.Stats { return enginetest.Stats{ HeadAge: st.HeadAge, WantedParts: st.WantedParts, Holes: st.Holes, LostParts: st.LostParts, - IndexFenced: st.IndexFenced, + IndexFenced: st.IndexFenced, TenureUnestablished: st.TenureUnestablished, } } diff --git a/engine/index.go b/engine/index.go index a5835125..6bb671c3 100644 --- a/engine/index.go +++ b/engine/index.go @@ -934,7 +934,7 @@ func (e *Engine) establishTenure(ctx context.Context) error { // refused — and the fresh load is what makes a predecessor commit that landed first this tenure's // starting point, rather than something a rebase of the view held as a replica would carry the // consumed inputs back in beside. A no-op once established, without a cluster, or without a claim. -// It is bounded like any commit, and until it lands every commit of the tenure is refused, so a +// It makes one attempt per call, and until one lands every commit of the tenure is refused, so a // flush keeps its rows in the head and the WAL. Caller holds e.flushMu and e.mu. func (e *Engine) establishTenureLocked(ctx context.Context) error { if e.cfg.Term == nil || e.cfg.Backend == nil { @@ -946,26 +946,45 @@ func (e *Engine) establishTenureLocked(ctx context.Context) error { return nil } - for range indexCommitAttempts { - if err := e.loadPartsLocked(ctx, loadReplica); err != nil { - return errors.Wrap(err, "load bucket index to establish the tenure") - } + // One attempt, not a loop: a failure leaves the engine exactly where a failed flush would, and + // the next flush or merge tries again, so a contended or unreachable index costs one load and one + // CAS per operation instead of holding the locks through a retry loop. + if err := e.loadPartsLocked(ctx, loadReplica); err != nil { + e.cfg.Obs.Corruption.EstablishFailed(ctx, metricSignal, obs.EstablishUnavailable) - if err := bucketindex.CheckTenure(term, e.term(), e.generation.Term); err != nil { - return err - } + return errors.Wrap(err, "load bucket index to establish the tenure") + } - landed, err := e.commitOnceLocked(ctx) - if err != nil { - return err - } + if err := bucketindex.CheckTenure(term, e.term(), e.generation.Term); err != nil { + return err + } - if landed { - e.established = term + landed, err := e.commitOnceLocked(ctx) + if err != nil { + e.cfg.Obs.Corruption.EstablishFailed(ctx, metricSignal, obs.EstablishUnavailable) - return nil - } + return errors.Wrapf(err, "establish term %d", term) + } + + if !landed { + e.cfg.Obs.Corruption.EstablishFailed(ctx, metricSignal, obs.EstablishConflict) + + return errors.Wrapf(bucketindex.ErrConflict, "establish term %d", term) } - return errors.Wrapf(bucketindex.ErrConflict, "establish term %d after %d attempts", term, indexCommitAttempts) + e.established = term + + return nil +} + +// tenureUnestablishedLocked reports a clustered engine that holds a claim whose tenure has not yet +// made its first commit, so it commits nothing. Caller holds e.mu. +func (e *Engine) tenureUnestablishedLocked() bool { + if e.cfg.Term == nil || e.cfg.Backend == nil { + return false + } + + term := e.term() + + return term != 0 && e.established != term } diff --git a/inspect.go b/inspect.go index a06958ec..cbebd6cf 100644 --- a/inspect.go +++ b/inspect.go @@ -143,6 +143,12 @@ type SignalStats struct { // IndexLoadError is the error of the failed load behind IndexFenced: which part or index, and // why. Empty while not fenced. IndexLoadError string + // TenureUnestablished is true while this node holds the shard's claim but its tenure's first + // index commit has not landed (storage.index.establish_failures counts the attempts). Like a + // fenced engine it commits nothing, so the head keeps its records and grows; every flush and + // merge tries again. It is set for a moment after every handoff; one that stays set is an index + // the node cannot load or commit to. + TenureUnestablished bool } // ClusterStats is the cluster-mode view of this node. @@ -276,6 +282,7 @@ func (s *Storage) Inspect() StoreStats { WAL: hasWAL, WALSegments: segs, WALBytes: walBytes, WALEpoch: epoch, WantedParts: es.WantedParts, Holes: es.Holes, LostParts: es.LostParts, IndexFenced: es.IndexFenced, IndexLoadError: errText(es.IndexLoadErr), + TenureUnestablished: es.TenureUnestablished, }) s.attachReadGap(&ts.Signals[len(ts.Signals)-1], signal.Metric, tid) @@ -307,6 +314,7 @@ func (s *Storage) Inspect() StoreStats { WAL: hasWAL, WALSegments: segs, WALBytes: walBytes, WALEpoch: epoch, WantedParts: es.WantedParts, Holes: es.Holes, LostParts: es.LostParts, IndexFenced: es.IndexFenced, IndexLoadError: errText(es.IndexLoadErr), + TenureUnestablished: es.TenureUnestablished, }) s.attachReadGap(&ts.Signals[len(ts.Signals)-1], sig, tid) } diff --git a/internal/enginetest/enginetest.go b/internal/enginetest/enginetest.go index da82dfe2..3086b339 100644 --- a/internal/enginetest/enginetest.go +++ b/internal/enginetest/enginetest.go @@ -57,6 +57,8 @@ type Stats struct { Holes int LostParts uint64 IndexFenced bool + // TenureUnestablished is a clustered engine whose tenure's first commit has not landed. + TenureUnestablished bool } // MergeShape is the subset of the engine's merge shape the suite asserts on. diff --git a/internal/enginetest/tenurefence.go b/internal/enginetest/tenurefence.go index 53477e3f..b0533c76 100644 --- a/internal/enginetest/tenurefence.go +++ b/internal/enginetest/tenurefence.go @@ -12,6 +12,8 @@ import ( "github.com/oteldb/storage/backend" "github.com/oteldb/storage/backend/bucketindex" "github.com/oteldb/storage/backend/faultbackend" + "github.com/oteldb/storage/internal/obs" + "github.com/oteldb/storage/internal/obs/obstest" ) // staleCommitCannotLandAfterTheSuccessorEstablishes is the check-to-CAS window: A's merge passes its @@ -118,31 +120,65 @@ func tenuresAllocateDisjointBlocks(t *testing.T, k Kind) { } // unestablishedTenureKeepsRowsInTheHead: until a new tenure's first commit lands, nothing of it -// commits. A flush whose establishing commit cannot land fails without writing a part, keeps its rows -// in the head, and the next flush establishes the tenure and publishes them. +// commits. Each flush and merge makes one attempt — one CAS, no retry loop — and one that cannot land, +// whether it lost the race or the backend failed, is counted, fails without writing a part and keeps +// the rows readable in the head. The first attempt that lands establishes the tenure and the same +// flush publishes every held row. func unestablishedTenureKeepsRowsInTheHead(t *testing.T, k Kind) { t.Helper() + const cycles = 3 + ctx := context.Background() inner := backend.Memory() be := faultbackend.Wrap(inner) + o, m := obstest.New(t) + + e := k.Open(t, Config{Backend: be, Term: termOf(ownerTerm), Obs: o}) + require.NoError(t, e.LoadParts(ctx)) + + held := []Row{api(100, 1), api(200, 2)} + e.Append(t, held...) - e := k.openTenure(t, be, ownerTerm, 0) - e.Append(t, api(100, 1)) + commits := func() int { return be.Count(k.indexCommit) } + + be.Add(faultbackend.Rule{Kind: faultbackend.CompareAndSwap, Lose: true}) + + for i := range cycles { + before := commits() + require.ErrorIsf(t, e.Flush(ctx), bucketindex.ErrConflict, "cycle %d", i) + require.ErrorIsf(t, e.ForceMerge(ctx), bucketindex.ErrConflict, "cycle %d", i) + assert.Equal(t, before+2, commits(), "one attempt per flush and one per merge") + } + assert.EqualValues(t, 2*cycles, + m.Counter("storage.index.establish_failures", "reason", obs.EstablishConflict)) + + be.Reset() be.Add(faultbackend.Rule{Kind: faultbackend.CompareAndSwap, Err: assert.AnError}) - require.ErrorIs(t, e.Flush(ctx), assert.AnError) + + for range cycles { + require.ErrorIs(t, e.Flush(ctx), assert.AnError) + } + + assert.EqualValues(t, cycles, + m.Counter("storage.index.establish_failures", "reason", obs.EstablishUnavailable)) + assert.True(t, e.Stats().TenureUnestablished) assert.Empty(t, k.partDirs(ctx, t, inner), "no part written for a tenure that could not establish") - assert.Equal(t, 1, e.HeadRows()) + assert.Equal(t, len(held), e.HeadRows()) + assert.Equal(t, held, sortedRows(t, e), "the held rows stay readable from the head") be.Reset() + e.Append(t, api(300, 3)) require.NoError(t, e.Flush(ctx)) ix := k.loadIndex(t, inner) require.Len(t, ix.Entries, 1) assert.Equal(t, bucketindex.Generation{Term: ownerTerm, Counter: 2}, ix.Generation, "the establishing commit, then the flush") - assert.Equal(t, []Row{api(100, 1)}, sortedRows(t, e)) + assert.False(t, e.Stats().TenureUnestablished) + assert.Zero(t, e.HeadRows()) + assert.Equal(t, []Row{api(100, 1), api(200, 2), api(300, 3)}, sortedRows(t, e)) } // writingTermSurvivesTheNextTenure pins that a part's writing term is part of its identity, not of diff --git a/internal/obs/corruption.go b/internal/obs/corruption.go index 89c08b61..12be55cc 100644 --- a/internal/obs/corruption.go +++ b/internal/obs/corruption.go @@ -28,12 +28,23 @@ const ( FenceUnavailable = "unavailable" ) -// Corruption instruments on-disk artifacts that failed their integrity checks. It is counted where -// the corruption is handled rather than where a codec detects it: only the consuming layer knows -// whether the engine fell back or failed. +// Reasons a tenure's establishing commit failed, the reason attribute of +// storage.index.establish_failures. +const ( + // EstablishConflict is a commit that lost the index CAS to another writer. + EstablishConflict = "conflict" + // EstablishUnavailable is a load or commit the backend failed to answer. + EstablishUnavailable = "unavailable" +) + +// Corruption instruments on-disk artifacts that failed their integrity checks, and the index +// commits an engine is refusing because of them or of its tenure. It is counted where the failure +// is handled rather than where a codec detects it: only the consuming layer knows whether the engine +// fell back or failed. type Corruption struct { - detected metric.Int64Counter - fencedLoads metric.Int64Counter + detected metric.Int64Counter + fencedLoads metric.Int64Counter + establishFailures metric.Int64Counter } // Detected accounts one corrupt artifact: component names what failed its check (wal, part, @@ -52,6 +63,14 @@ func (c *Corruption) FencedLoad(ctx context.Context, sig, reason string) { attribute.String("signal", sig), attribute.String("reason", reason))) } +// EstablishFailed accounts one attempt of an engine of sig to make its tenure's first commit that +// did not land, for reason ([EstablishConflict] or [EstablishUnavailable]). Until one lands the +// engine commits nothing, and it tries once per flush and once per merge. +func (c *Corruption) EstablishFailed(ctx context.Context, sig, reason string) { + c.establishFailures.Add(ctx, 1, metric.WithAttributes( + attribute.String("signal", sig), attribute.String("reason", reason))) +} + func newCorruption(m metric.Meter) (*Corruption, error) { b := &imb{m: m} c := &Corruption{ @@ -59,6 +78,8 @@ func newCorruption(m metric.Meter) (*Corruption, error) { "on-disk artifacts that failed an integrity check, by component and disposition", "{artifact}"), fencedLoads: b.counter("storage.index.fenced_loads", "index loads that failed and left the engine refusing commits until a load succeeds", "{load}"), + establishFailures: b.counter("storage.index.establish_failures", + "attempts to make a new tenure's first index commit that did not land", "{attempt}"), } return c, b.err diff --git a/recordengine/engine.go b/recordengine/engine.go index 712306dc..67b13b40 100644 --- a/recordengine/engine.go +++ b/recordengine/engine.go @@ -491,6 +491,10 @@ type Stats struct { IndexFenced bool // IndexLoadErr is the error of the failed load behind IndexFenced, nil while not fenced. IndexLoadErr error + // TenureUnestablished is set while this clustered engine holds the shard's claim but its + // tenure's first commit has not landed: it commits nothing, so flushes keep their rows in the + // head, and every flush and merge tries again. + TenureUnestablished bool } // Stats returns an in-memory snapshot of the engine's state under a single read lock (no backend @@ -511,7 +515,9 @@ func (e *Engine) Stats() Stats { LostParts: e.lostParts, IndexFenced: e.loadErr != nil, IndexLoadErr: e.loadErr, - MaxTime: e.head.newest, + + TenureUnestablished: e.tenureUnestablishedLocked(), + MaxTime: e.head.newest, } for _, buf := range e.head.records { diff --git a/recordengine/enginetest_test.go b/recordengine/enginetest_test.go index e3db2528..3f91beca 100644 --- a/recordengine/enginetest_test.go +++ b/recordengine/enginetest_test.go @@ -93,7 +93,7 @@ func (e recordEngine) Stats() enginetest.Stats { return enginetest.Stats{ HeadAge: st.HeadAge, WantedParts: st.WantedParts, Holes: st.Holes, LostParts: st.LostParts, - IndexFenced: st.IndexFenced, + IndexFenced: st.IndexFenced, TenureUnestablished: st.TenureUnestablished, } } diff --git a/recordengine/index.go b/recordengine/index.go index 325e9f6a..cbc05ecc 100644 --- a/recordengine/index.go +++ b/recordengine/index.go @@ -948,7 +948,7 @@ func (e *Engine) establishTenure(ctx context.Context) error { // refused — and the fresh load is what makes a predecessor commit that landed first this tenure's // starting point, rather than something a rebase of the view held as a replica would carry the // consumed inputs back in beside. A no-op once established, without a cluster, or without a claim. -// It is bounded like any commit, and until it lands every commit of the tenure is refused, so a +// It makes one attempt per call, and until one lands every commit of the tenure is refused, so a // flush keeps its rows in the head and the WAL. Caller holds e.flushMu and e.mu. func (e *Engine) establishTenureLocked(ctx context.Context) error { if e.cfg.Term == nil || e.cfg.Backend == nil { @@ -960,26 +960,45 @@ func (e *Engine) establishTenureLocked(ctx context.Context) error { return nil } - for range indexCommitAttempts { - if err := e.loadPartsLocked(ctx, loadReplica); err != nil { - return errors.Wrap(err, "load bucket index to establish the tenure") - } + // One attempt, not a loop: a failure leaves the engine exactly where a failed flush would, and + // the next flush or merge tries again, so a contended or unreachable index costs one load and one + // CAS per operation instead of holding the locks through a retry loop. + if err := e.loadPartsLocked(ctx, loadReplica); err != nil { + e.cfg.Obs.Corruption.EstablishFailed(ctx, e.cfg.Signal, obs.EstablishUnavailable) - if err := bucketindex.CheckTenure(term, e.term(), e.generation.Term); err != nil { - return err - } + return errors.Wrap(err, "load bucket index to establish the tenure") + } - landed, err := e.commitOnceLocked(ctx) - if err != nil { - return err - } + if err := bucketindex.CheckTenure(term, e.term(), e.generation.Term); err != nil { + return err + } - if landed { - e.established = term + landed, err := e.commitOnceLocked(ctx) + if err != nil { + e.cfg.Obs.Corruption.EstablishFailed(ctx, e.cfg.Signal, obs.EstablishUnavailable) - return nil - } + return errors.Wrapf(err, "establish term %d", term) + } + + if !landed { + e.cfg.Obs.Corruption.EstablishFailed(ctx, e.cfg.Signal, obs.EstablishConflict) + + return errors.Wrapf(bucketindex.ErrConflict, "establish term %d", term) } - return errors.Wrapf(bucketindex.ErrConflict, "establish term %d after %d attempts", term, indexCommitAttempts) + e.established = term + + return nil +} + +// tenureUnestablishedLocked reports a clustered engine that holds a claim whose tenure has not yet +// made its first commit, so it commits nothing. Caller holds e.mu. +func (e *Engine) tenureUnestablishedLocked() bool { + if e.cfg.Term == nil || e.cfg.Backend == nil { + return false + } + + term := e.term() + + return term != 0 && e.established != term } From 73c7519296a096ae61021f533157de5e9225f1f6 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Mon, 28 Sep 2026 23:58:52 +0300 Subject: [PATCH 8/8] test(storage): wait for the handoff to converge instead of three fixed cycles A new tenure now makes one establishing attempt per flush and merge, so a slow runner can need more cycles before the old owner has taken the shard back and merged. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KcLq1G8KpWBvM53JaQb1tS --- cluster_tenure_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cluster_tenure_test.go b/cluster_tenure_test.go index b7621439..2c76dd52 100644 --- a/cluster_tenure_test.go +++ b/cluster_tenure_test.go @@ -104,11 +104,15 @@ func TestClusterHandoffUnsyncedFlushReadsOnce(t *testing.T) { }, 30*time.Second, 20*time.Millisecond, "the old owner rejoins") // The old owner installs the new owner's index beside the part it holds, takes the shard back once - // the secondary releases it, and merges; the secondary mirrors the result. - for range 3 { + // the secondary releases it, and merges; the secondary mirrors the result. How many cycles that + // takes is timing: a new tenure makes one establishing attempt per flush and merge, and one that + // meets a concurrent index write waits for the next cycle. + require.Eventually(t, func() bool { oldOwner.maintain(ctx) secondary.maintain(ctx) - } + + return oldOwner.claimsShard(shard) && len(om.Parts()) == 1 && len(sm.Parts()) == 1 + }, 30*time.Second, 20*time.Millisecond, "the old owner takes the shard back, merges, and the secondary mirrors it") // The metric half does not duplicate: a read merges a series by timestamp, and so does the merge // that folds the two parts together.