test(tree): cover freeze-then-continue and concurrent snapshot readers
- commit
- 15cf399f20b059165b3b61ab1cf106e61b2d001d
- author
- brice · · 2026-10-10 01:57 UTC
- parent
- f361506
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +68 -6
M tree.go +7 -6
| @@ -276,17 +276,18 @@ func (b *builder) delete(key []byte) { | |||
| 276 | 276 | } | |
| 277 | 277 | } | |
| 278 | 278 | ||
| 279 | - | // remove deletes key (or, with max, the largest entry) from the subtree of the | |
| 280 | - | // owned node n and returns the removed entry. The key must exist. Every child | |
| 281 | - | // descended into is first guaranteed to hold more than minEntries entries. | |
| 282 | - | func (b *builder) remove(n *node, key []byte, max bool) entry { | |
| 279 | + | // remove deletes key (or, with rightmost, the largest entry) from the subtree | |
| 280 | + | // of the owned node n and returns the removed entry. The key must exist: a | |
| 281 | + | // missing key would delete an unrelated leaf entry. Every child descended into | |
| 282 | + | // is first guaranteed to hold more than minEntries entries. | |
| 283 | + | func (b *builder) remove(n *node, key []byte, rightmost bool) entry { | |
| 283 | 284 | for { | |
| 284 | 285 | var i int | |
| 285 | 286 | var found bool | |
| 286 | 287 | switch { | |
| 287 | - | case max && n.leaf(): | |
| 288 | + | case rightmost && n.leaf(): | |
| 288 | 289 | i, found = len(n.entries)-1, true | |
| 289 | - | case max: | |
| 290 | + | case rightmost: | |
| 290 | 291 | i = len(n.entries) | |
| 291 | 292 | default: | |
| 292 | 293 | i, found = n.find(key) | |
| @@ -276,17 +276,18 @@ func (b *builder) delete(key []byte) { | |||||
| 276 | } | 276 | } | ||
| 277 | } | 277 | } | ||
| 278 | 278 | ||||
| 279 | - | // remove deletes key (or, with max, the largest entry) from the subtree of the | 279 | + | // remove deletes key (or, with rightmost, the largest entry) from the subtree |
| 280 | - | // owned node n and returns the removed entry. The key must exist. Every child | 280 | + | // of the owned node n and returns the removed entry. The key must exist: a |
| 281 | - | // descended into is first guaranteed to hold more than minEntries entries. | 281 | + | // missing key would delete an unrelated leaf entry. Every child descended into |
| 282 | - | func (b *builder) remove(n *node, key []byte, max bool) entry { | 282 | + | // is first guaranteed to hold more than minEntries entries. |
| 283 | + | func (b *builder) remove(n *node, key []byte, rightmost bool) entry { | |||
| 283 | for { | 284 | for { | ||
| 284 | var i int | 285 | var i int | ||
| 285 | var found bool | 286 | var found bool | ||
| 286 | switch { | 287 | switch { | ||
| 287 | - | case max && n.leaf(): | 288 | + | case rightmost && n.leaf(): |
| 288 | i, found = len(n.entries)-1, true | 289 | i, found = len(n.entries)-1, true | ||
| 289 | - | case max: | 290 | + | case rightmost: |
| 290 | i = len(n.entries) | 291 | i = len(n.entries) | ||
| 291 | default: | 292 | default: | ||
| 292 | i, found = n.find(key) | 293 | i, found = n.find(key) | ||
M tree_test.go +61 -0
| @@ -7,6 +7,8 @@ import ( | |||
| 7 | 7 | "fmt" | |
| 8 | 8 | "math/rand/v2" | |
| 9 | 9 | "slices" | |
| 10 | + | "sync" | |
| 11 | + | "sync/atomic" | |
| 10 | 12 | "testing" | |
| 11 | 13 | ) | |
| 12 | 14 | ||
| @@ -243,6 +245,11 @@ func TestTreeModel(t *testing.T) { | |||
| 243 | 245 | if got, ok := b.get(k); ok != (m[string(k)].Version != 0) || (ok && !equalItem(got, m[string(k)])) { | |
| 244 | 246 | t.Fatalf("seed %d: builder.get mismatch", seed) | |
| 245 | 247 | } | |
| 248 | + | if rng.IntN(512) == 0 { | |
| 249 | + | // Freeze mid-group and keep using the builder: the | |
| 250 | + | // frozen tree must never see later builder writes. | |
| 251 | + | history = append(history, frozen{b.freeze(), m.clone()}) | |
| 252 | + | } | |
| 246 | 253 | } | |
| 247 | 254 | if b.len() != len(m) { | |
| 248 | 255 | t.Fatalf("seed %d: builder.len = %d, want %d", seed, b.len(), len(m)) | |
| @@ -444,3 +451,57 @@ func TestTreeDeep(t *testing.T) { | |||
| 444 | 451 | } | |
| 445 | 452 | checkStructure(t, full) | |
| 446 | 453 | } | |
| 454 | + | ||
| 455 | + | // TestConcurrentReaders reads published snapshots without locks while one | |
| 456 | + | // builder keeps writing and freezing, so -race sees any shared mutation. | |
| 457 | + | func TestConcurrentReaders(t *testing.T) { | |
| 458 | + | var cur atomic.Pointer[Snapshot] | |
| 459 | + | cur.Store(newSnapshot(tree{}, 0)) | |
| 460 | + | stop := make(chan struct{}) | |
| 461 | + | var wg sync.WaitGroup | |
| 462 | + | for range 8 { | |
| 463 | + | wg.Go(func() { | |
| 464 | + | for { | |
| 465 | + | select { | |
| 466 | + | case <-stop: | |
| 467 | + | return | |
| 468 | + | default: | |
| 469 | + | } | |
| 470 | + | s := cur.Load() | |
| 471 | + | n := 0 | |
| 472 | + | var prev []byte | |
| 473 | + | for k, it := range s.Range(nil, nil) { | |
| 474 | + | if prev != nil && bytes.Compare(prev, k) >= 0 { | |
| 475 | + | t.Errorf("seq %d: keys out of order", s.Seq()) | |
| 476 | + | return | |
| 477 | + | } | |
| 478 | + | if it.Version > s.Seq() { | |
| 479 | + | t.Errorf("seq %d: version %d from the future", s.Seq(), it.Version) | |
| 480 | + | return | |
| 481 | + | } | |
| 482 | + | prev = k | |
| 483 | + | n++ | |
| 484 | + | } | |
| 485 | + | if n != s.Len() { | |
| 486 | + | t.Errorf("seq %d: ranged %d keys, Len %d", s.Seq(), n, s.Len()) | |
| 487 | + | return | |
| 488 | + | } | |
| 489 | + | } | |
| 490 | + | }) | |
| 491 | + | } | |
| 492 | + | rng := rand.New(rand.NewPCG(1, 2)) | |
| 493 | + | b := cur.Load().t.builder() | |
| 494 | + | for seq := uint64(1); seq <= 3000; seq++ { | |
| 495 | + | for range 1 + rng.IntN(8) { | |
| 496 | + | k := fmt.Appendf(nil, "k%04d", rng.IntN(2000)) | |
| 497 | + | if rng.IntN(3) == 0 { | |
| 498 | + | b.delete(k) | |
| 499 | + | } else { | |
| 500 | + | b.put(k, []byte("v"), seq) | |
| 501 | + | } | |
| 502 | + | } | |
| 503 | + | cur.Store(newSnapshot(b.freeze(), seq)) | |
| 504 | + | } | |
| 505 | + | close(stop) | |
| 506 | + | wg.Wait() | |
| 507 | + | } | |
| @@ -7,6 +7,8 @@ import ( | |||||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | "math/rand/v2" | 8 | "math/rand/v2" | ||
| 9 | "slices" | 9 | "slices" | ||
| 10 | + | "sync" | |||
| 11 | + | "sync/atomic" | |||
| 10 | "testing" | 12 | "testing" | ||
| 11 | ) | 13 | ) | ||
| 12 | 14 | ||||
| @@ -243,6 +245,11 @@ func TestTreeModel(t *testing.T) { | |||||
| 243 | if got, ok := b.get(k); ok != (m[string(k)].Version != 0) || (ok && !equalItem(got, m[string(k)])) { | 245 | if got, ok := b.get(k); ok != (m[string(k)].Version != 0) || (ok && !equalItem(got, m[string(k)])) { | ||
| 244 | t.Fatalf("seed %d: builder.get mismatch", seed) | 246 | t.Fatalf("seed %d: builder.get mismatch", seed) | ||
| 245 | } | 247 | } | ||
| 248 | + | if rng.IntN(512) == 0 { | |||
| 249 | + | // Freeze mid-group and keep using the builder: the | |||
| 250 | + | // frozen tree must never see later builder writes. | |||
| 251 | + | history = append(history, frozen{b.freeze(), m.clone()}) | |||
| 252 | + | } | |||
| 246 | } | 253 | } | ||
| 247 | if b.len() != len(m) { | 254 | if b.len() != len(m) { | ||
| 248 | t.Fatalf("seed %d: builder.len = %d, want %d", seed, b.len(), len(m)) | 255 | t.Fatalf("seed %d: builder.len = %d, want %d", seed, b.len(), len(m)) | ||
| @@ -444,3 +451,57 @@ func TestTreeDeep(t *testing.T) { | |||||
| 444 | } | 451 | } | ||
| 445 | checkStructure(t, full) | 452 | checkStructure(t, full) | ||
| 446 | } | 453 | } | ||
| 454 | + | ||||
| 455 | + | // TestConcurrentReaders reads published snapshots without locks while one | |||
| 456 | + | // builder keeps writing and freezing, so -race sees any shared mutation. | |||
| 457 | + | func TestConcurrentReaders(t *testing.T) { | |||
| 458 | + | var cur atomic.Pointer[Snapshot] | |||
| 459 | + | cur.Store(newSnapshot(tree{}, 0)) | |||
| 460 | + | stop := make(chan struct{}) | |||
| 461 | + | var wg sync.WaitGroup | |||
| 462 | + | for range 8 { | |||
| 463 | + | wg.Go(func() { | |||
| 464 | + | for { | |||
| 465 | + | select { | |||
| 466 | + | case <-stop: | |||
| 467 | + | return | |||
| 468 | + | default: | |||
| 469 | + | } | |||
| 470 | + | s := cur.Load() | |||
| 471 | + | n := 0 | |||
| 472 | + | var prev []byte | |||
| 473 | + | for k, it := range s.Range(nil, nil) { | |||
| 474 | + | if prev != nil && bytes.Compare(prev, k) >= 0 { | |||
| 475 | + | t.Errorf("seq %d: keys out of order", s.Seq()) | |||
| 476 | + | return | |||
| 477 | + | } | |||
| 478 | + | if it.Version > s.Seq() { | |||
| 479 | + | t.Errorf("seq %d: version %d from the future", s.Seq(), it.Version) | |||
| 480 | + | return | |||
| 481 | + | } | |||
| 482 | + | prev = k | |||
| 483 | + | n++ | |||
| 484 | + | } | |||
| 485 | + | if n != s.Len() { | |||
| 486 | + | t.Errorf("seq %d: ranged %d keys, Len %d", s.Seq(), n, s.Len()) | |||
| 487 | + | return | |||
| 488 | + | } | |||
| 489 | + | } | |||
| 490 | + | }) | |||
| 491 | + | } | |||
| 492 | + | rng := rand.New(rand.NewPCG(1, 2)) | |||
| 493 | + | b := cur.Load().t.builder() | |||
| 494 | + | for seq := uint64(1); seq <= 3000; seq++ { | |||
| 495 | + | for range 1 + rng.IntN(8) { | |||
| 496 | + | k := fmt.Appendf(nil, "k%04d", rng.IntN(2000)) | |||
| 497 | + | if rng.IntN(3) == 0 { | |||
| 498 | + | b.delete(k) | |||
| 499 | + | } else { | |||
| 500 | + | b.put(k, []byte("v"), seq) | |||
| 501 | + | } | |||
| 502 | + | } | |||
| 503 | + | cur.Store(newSnapshot(b.freeze(), seq)) | |||
| 504 | + | } | |||
| 505 | + | close(stop) | |||
| 506 | + | wg.Wait() | |||
| 507 | + | } | |||