detached 19d4f5e

fix(db): make failure win over close in Apply and group tests deterministic

commit
19d4f5eb6ce7f2675ee9ae3ae47c427c205a1e8e
author
brice · · 2026-10-10 02:51 UTC
parent
2de317d
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
3 files · +68 -29
M db.go +20 -8
@@ -217,12 +217,8 @@ func (db *DB) Apply(ctx context.Context, b *Batch) (uint64, error) {
217217 if b.encodedSize() > db.opts.MaxBatchBytes {
218218 return 0, ErrBatchTooLarge
219219 }
220- select {
221- case <-db.closing:
222- return 0, ErrClosed
223- case <-db.failed:
224- return 0, db.failErr
225- default:
220+ if err := db.stopped(); err != nil {
221+ return 0, err
226222 }
227223 if err := ctx.Err(); err != nil {
228224 return 0, err
@@ -235,11 +231,27 @@ func (db *DB) Apply(ctx context.Context, b *Batch) (uint64, error) {
235231 case <-ctx.Done():
236232 return 0, r.release(ctx.Err())
237233 case <-db.closing:
238- return 0, r.release(ErrClosed)
234+ return 0, r.release(db.stopped())
239235 case <-db.failed:
240- return 0, r.release(db.failErr)
236+ return 0, r.release(db.stopped())
241237 }
242238 res := <-r.reply // the writer always answers a request it has taken
243239 _ = r.release(nil)
244240 return res.seq, res.err
245241 }
242+
243+// stopped returns the failure (which wins over Close, so callers see a
244+// stable error), ErrClosed, or nil while the database accepts batches.
245+func (db *DB) stopped() error {
246+ select {
247+ case <-db.failed:
248+ return db.failErr
249+ default:
250+ }
251+ select {
252+ case <-db.closing:
253+ return ErrClosed
254+ default:
255+ return nil
256+ }
257+}
@@ -217,12 +217,8 @@ func (db *DB) Apply(ctx context.Context, b *Batch) (uint64, error) {
217 if b.encodedSize() > db.opts.MaxBatchBytes {217 if b.encodedSize() > db.opts.MaxBatchBytes {
218 return 0, ErrBatchTooLarge218 return 0, ErrBatchTooLarge
219 }219 }
220- select {220+ if err := db.stopped(); err != nil {
221- case <-db.closing:221+ return 0, err
222- return 0, ErrClosed
223- case <-db.failed:
224- return 0, db.failErr
225- default:
226 }222 }
227 if err := ctx.Err(); err != nil {223 if err := ctx.Err(); err != nil {
228 return 0, err224 return 0, err
@@ -235,11 +231,27 @@ func (db *DB) Apply(ctx context.Context, b *Batch) (uint64, error) {
235 case <-ctx.Done():231 case <-ctx.Done():
236 return 0, r.release(ctx.Err())232 return 0, r.release(ctx.Err())
237 case <-db.closing:233 case <-db.closing:
238- return 0, r.release(ErrClosed)234+ return 0, r.release(db.stopped())
239 case <-db.failed:235 case <-db.failed:
240- return 0, r.release(db.failErr)236+ return 0, r.release(db.stopped())
241 }237 }
242 res := <-r.reply // the writer always answers a request it has taken238 res := <-r.reply // the writer always answers a request it has taken
243 _ = r.release(nil)239 _ = r.release(nil)
244 return res.seq, res.err240 return res.seq, res.err
245 }241 }
242+
243+// stopped returns the failure (which wins over Close, so callers see a
244+// stable error), ErrClosed, or nil while the database accepts batches.
245+func (db *DB) stopped() error {
246+ select {
247+ case <-db.failed:
248+ return db.failErr
249+ default:
250+ }
251+ select {
252+ case <-db.closing:
253+ return ErrClosed
254+ default:
255+ return nil
256+ }
257+}
M db_test.go +45 -16
@@ -351,30 +351,57 @@ func TestPreconditionsBeforeOps(t *testing.T) {
351351 mustMissing(t, db.Snapshot(bg), "other")
352352 }
353353
354+// holdWriter applies a batch whose sync is slow and returns once the writer
355+// is inside that sync, so batches applied next queue up and form one group
356+// (GroupWindow 0 drains everything already queued). wait returns when the
357+// held batch is acknowledged; the sync delay stays set until then.
358+func holdWriter(t *testing.T, db *DB, fx *faultfs.FS) (wait func()) {
359+ t.Helper()
360+ base := fx.Syncs()
361+ fx.SetSyncDelay(300 * time.Millisecond)
362+ done := make(chan error, 1)
363+ go func() {
364+ var b Batch
365+ b.Put([]byte("hold"), nil)
366+ _, err := db.Apply(bg, &b)
367+ done <- err
368+ }()
369+ for fx.Syncs() == base {
370+ time.Sleep(time.Millisecond)
371+ }
372+ return func() {
373+ t.Helper()
374+ if err := <-done; err != nil {
375+ t.Fatalf("held Apply: %v", err)
376+ }
377+ fx.SetSyncDelay(0)
378+ }
379+}
380+
354381 // TestGroupPreconditions checks that preconditions see earlier batches of
355-// the same group: with a group window, the second batch must conflict.
382+// the same group: the second batch of the group must conflict.
356383 func TestGroupPreconditions(t *testing.T) {
357384 fx := faultfs.New()
358- db := mustOpen(t, fx, Options{GroupWindow: 200 * time.Millisecond})
385+ db := mustOpen(t, fx, Options{})
359386 syncs := fx.Syncs()
387+ wait := holdWriter(t, db, fx)
360388 errs := make([]error, 2)
361389 var wg sync.WaitGroup
362390 for i := range errs {
363- wg.Add(1)
364- go func() {
365- defer wg.Done()
391+ wg.Go(func() {
366392 var b Batch
367393 b.RequireAbsent([]byte("k"))
368394 b.Put([]byte("k"), []byte{byte('a' + i)})
369395 _, errs[i] = db.Apply(bg, &b)
370- }()
396+ })
371397 }
372398 wg.Wait()
399+ wait()
373400 if (errs[0] == nil) == (errs[1] == nil) {
374401 t.Fatalf("errors %v, want exactly one success", errs)
375402 }
376- if fx.Syncs() != syncs+1 {
377- t.Fatalf("%d syncs for the group, want 1", fx.Syncs()-syncs)
403+ if got := fx.Syncs() - syncs; got != 2 {
404+ t.Fatalf("%d syncs, want 2 (held batch, then one group)", got)
378405 }
379406 }
380407
@@ -672,7 +699,7 @@ func TestContextBeforeDequeue(t *testing.T) {
672699 _, err := db.Apply(ctx, &b)
673700 second <- err
674701 }()
675- time.Sleep(10 * time.Millisecond)
702+ time.Sleep(50 * time.Millisecond) // let Apply reach its blocking select
676703 cancel()
677704 if err := <-second; !errors.Is(err, context.Canceled) {
678705 t.Fatalf("Apply = %v, want context.Canceled", err)
@@ -787,11 +814,11 @@ func testInjectedFailure(t *testing.T, f failure, powerLoss bool) {
787814 fx := faultfs.New()
788815 var logs syncBuffer
789816 ctx := zerowrap.WithCtx(bg, zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs}))
790- db := mustOpenCtx(t, ctx, fx, Options{GroupWindow: 150 * time.Millisecond})
791- putKV(t, db, "before", "1")
817+ db := mustOpenCtx(t, ctx, fx, Options{})
818+ wait := holdWriter(t, db, fx) // seq 1: "hold"; its write and sync have started
792819 f.inject(fx)
793820
794- // Three batches form the failed group (the window gathers them).
821+ // Three batches queue behind the held one and form the failed group.
795822 const group = 3
796823 errs := make([]error, group)
797824 var wg sync.WaitGroup
@@ -803,6 +830,7 @@ func testInjectedFailure(t *testing.T, f failure, powerLoss bool) {
803830 })
804831 }
805832 wg.Wait()
833+ wait()
806834 for i, err := range errs {
807835 if !errors.Is(err, ErrFailed) {
808836 t.Fatalf("batch %d: %v, want ErrFailed", i, err)
@@ -831,7 +859,7 @@ func testInjectedFailure(t *testing.T, f failure, powerLoss bool) {
831859 if s.Seq() != 1 || s.Len() != 1 {
832860 t.Fatalf("snapshot after failure: seq %d len %d", s.Seq(), s.Len())
833861 }
834- mustGet(t, s, "before", "1")
862+ mustGet(t, s, "hold", "")
835863 mustMissing(t, s, "failed0")
836864 if err := db.Close(bg); !errors.Is(err, ErrFailed) {
837865 t.Fatalf("Close = %v, want the failure", err)
@@ -844,7 +872,7 @@ func testInjectedFailure(t *testing.T, f failure, powerLoss bool) {
844872 fx.PowerLoss(faultfs.PowerLossOptions{})
845873 }
846874 s = mustOpen(t, fx, Options{}).Snapshot(bg)
847- mustGet(t, s, "before", "1")
875+ mustGet(t, s, "hold", "")
848876 present := 0
849877 for i := range group {
850878 if _, ok := s.Get(bg, fmt.Appendf(nil, "failed%d", i)); ok {
@@ -904,7 +932,8 @@ func (s *syncBuffer) String() string {
904932 return s.b.String()
905933 }
906934
907-// powerLossBase returns the first seed; ERMITE_KV_SEED reproduces a run.
935+// powerLossBase returns the first seed. ERMITE_KV_SEED replays the same
936+// parameters; goroutine scheduling still varies between runs.
908937 func powerLossBase(t *testing.T) uint64 {
909938 if v := os.Getenv("ERMITE_KV_SEED"); v != "" {
910939 seed, err := strconv.ParseUint(v, 10, 64)
@@ -946,7 +975,7 @@ func TestPowerLoss(t *testing.T) {
946975 for i := range iterations {
947976 seed := base + uint64(i)
948977 if !runPowerLoss(t, seed, m.opts) {
949- t.Fatalf("failed with seed %d (ERMITE_KV_SEED=%d reproduces it)", seed, seed)
978+ t.Fatalf("failed with seed %d (ERMITE_KV_SEED=%d replays its parameters)", seed, seed)
950979 }
951980 }
952981 })
@@ -351,30 +351,57 @@ func TestPreconditionsBeforeOps(t *testing.T) {
351 mustMissing(t, db.Snapshot(bg), "other")351 mustMissing(t, db.Snapshot(bg), "other")
352 }352 }
353 353
354+// holdWriter applies a batch whose sync is slow and returns once the writer
355+// is inside that sync, so batches applied next queue up and form one group
356+// (GroupWindow 0 drains everything already queued). wait returns when the
357+// held batch is acknowledged; the sync delay stays set until then.
358+func holdWriter(t *testing.T, db *DB, fx *faultfs.FS) (wait func()) {
359+ t.Helper()
360+ base := fx.Syncs()
361+ fx.SetSyncDelay(300 * time.Millisecond)
362+ done := make(chan error, 1)
363+ go func() {
364+ var b Batch
365+ b.Put([]byte("hold"), nil)
366+ _, err := db.Apply(bg, &b)
367+ done <- err
368+ }()
369+ for fx.Syncs() == base {
370+ time.Sleep(time.Millisecond)
371+ }
372+ return func() {
373+ t.Helper()
374+ if err := <-done; err != nil {
375+ t.Fatalf("held Apply: %v", err)
376+ }
377+ fx.SetSyncDelay(0)
378+ }
379+}
380+
354 // TestGroupPreconditions checks that preconditions see earlier batches of381 // TestGroupPreconditions checks that preconditions see earlier batches of
355-// the same group: with a group window, the second batch must conflict.382+// the same group: the second batch of the group must conflict.
356 func TestGroupPreconditions(t *testing.T) {383 func TestGroupPreconditions(t *testing.T) {
357 fx := faultfs.New()384 fx := faultfs.New()
358- db := mustOpen(t, fx, Options{GroupWindow: 200 * time.Millisecond})385+ db := mustOpen(t, fx, Options{})
359 syncs := fx.Syncs()386 syncs := fx.Syncs()
387+ wait := holdWriter(t, db, fx)
360 errs := make([]error, 2)388 errs := make([]error, 2)
361 var wg sync.WaitGroup389 var wg sync.WaitGroup
362 for i := range errs {390 for i := range errs {
363- wg.Add(1)391+ wg.Go(func() {
364- go func() {
365- defer wg.Done()
366 var b Batch392 var b Batch
367 b.RequireAbsent([]byte("k"))393 b.RequireAbsent([]byte("k"))
368 b.Put([]byte("k"), []byte{byte('a' + i)})394 b.Put([]byte("k"), []byte{byte('a' + i)})
369 _, errs[i] = db.Apply(bg, &b)395 _, errs[i] = db.Apply(bg, &b)
370- }()396+ })
371 }397 }
372 wg.Wait()398 wg.Wait()
399+ wait()
373 if (errs[0] == nil) == (errs[1] == nil) {400 if (errs[0] == nil) == (errs[1] == nil) {
374 t.Fatalf("errors %v, want exactly one success", errs)401 t.Fatalf("errors %v, want exactly one success", errs)
375 }402 }
376- if fx.Syncs() != syncs+1 {403+ if got := fx.Syncs() - syncs; got != 2 {
377- t.Fatalf("%d syncs for the group, want 1", fx.Syncs()-syncs)404+ t.Fatalf("%d syncs, want 2 (held batch, then one group)", got)
378 }405 }
379 }406 }
380 407
@@ -672,7 +699,7 @@ func TestContextBeforeDequeue(t *testing.T) {
672 _, err := db.Apply(ctx, &b)699 _, err := db.Apply(ctx, &b)
673 second <- err700 second <- err
674 }()701 }()
675- time.Sleep(10 * time.Millisecond)702+ time.Sleep(50 * time.Millisecond) // let Apply reach its blocking select
676 cancel()703 cancel()
677 if err := <-second; !errors.Is(err, context.Canceled) {704 if err := <-second; !errors.Is(err, context.Canceled) {
678 t.Fatalf("Apply = %v, want context.Canceled", err)705 t.Fatalf("Apply = %v, want context.Canceled", err)
@@ -787,11 +814,11 @@ func testInjectedFailure(t *testing.T, f failure, powerLoss bool) {
787 fx := faultfs.New()814 fx := faultfs.New()
788 var logs syncBuffer815 var logs syncBuffer
789 ctx := zerowrap.WithCtx(bg, zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs}))816 ctx := zerowrap.WithCtx(bg, zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs}))
790- db := mustOpenCtx(t, ctx, fx, Options{GroupWindow: 150 * time.Millisecond})817+ db := mustOpenCtx(t, ctx, fx, Options{})
791- putKV(t, db, "before", "1")818+ wait := holdWriter(t, db, fx) // seq 1: "hold"; its write and sync have started
792 f.inject(fx)819 f.inject(fx)
793 820
794- // Three batches form the failed group (the window gathers them).821+ // Three batches queue behind the held one and form the failed group.
795 const group = 3822 const group = 3
796 errs := make([]error, group)823 errs := make([]error, group)
797 var wg sync.WaitGroup824 var wg sync.WaitGroup
@@ -803,6 +830,7 @@ func testInjectedFailure(t *testing.T, f failure, powerLoss bool) {
803 })830 })
804 }831 }
805 wg.Wait()832 wg.Wait()
833+ wait()
806 for i, err := range errs {834 for i, err := range errs {
807 if !errors.Is(err, ErrFailed) {835 if !errors.Is(err, ErrFailed) {
808 t.Fatalf("batch %d: %v, want ErrFailed", i, err)836 t.Fatalf("batch %d: %v, want ErrFailed", i, err)
@@ -831,7 +859,7 @@ func testInjectedFailure(t *testing.T, f failure, powerLoss bool) {
831 if s.Seq() != 1 || s.Len() != 1 {859 if s.Seq() != 1 || s.Len() != 1 {
832 t.Fatalf("snapshot after failure: seq %d len %d", s.Seq(), s.Len())860 t.Fatalf("snapshot after failure: seq %d len %d", s.Seq(), s.Len())
833 }861 }
834- mustGet(t, s, "before", "1")862+ mustGet(t, s, "hold", "")
835 mustMissing(t, s, "failed0")863 mustMissing(t, s, "failed0")
836 if err := db.Close(bg); !errors.Is(err, ErrFailed) {864 if err := db.Close(bg); !errors.Is(err, ErrFailed) {
837 t.Fatalf("Close = %v, want the failure", err)865 t.Fatalf("Close = %v, want the failure", err)
@@ -844,7 +872,7 @@ func testInjectedFailure(t *testing.T, f failure, powerLoss bool) {
844 fx.PowerLoss(faultfs.PowerLossOptions{})872 fx.PowerLoss(faultfs.PowerLossOptions{})
845 }873 }
846 s = mustOpen(t, fx, Options{}).Snapshot(bg)874 s = mustOpen(t, fx, Options{}).Snapshot(bg)
847- mustGet(t, s, "before", "1")875+ mustGet(t, s, "hold", "")
848 present := 0876 present := 0
849 for i := range group {877 for i := range group {
850 if _, ok := s.Get(bg, fmt.Appendf(nil, "failed%d", i)); ok {878 if _, ok := s.Get(bg, fmt.Appendf(nil, "failed%d", i)); ok {
@@ -904,7 +932,8 @@ func (s *syncBuffer) String() string {
904 return s.b.String()932 return s.b.String()
905 }933 }
906 934
907-// powerLossBase returns the first seed; ERMITE_KV_SEED reproduces a run.935+// powerLossBase returns the first seed. ERMITE_KV_SEED replays the same
936+// parameters; goroutine scheduling still varies between runs.
908 func powerLossBase(t *testing.T) uint64 {937 func powerLossBase(t *testing.T) uint64 {
909 if v := os.Getenv("ERMITE_KV_SEED"); v != "" {938 if v := os.Getenv("ERMITE_KV_SEED"); v != "" {
910 seed, err := strconv.ParseUint(v, 10, 64)939 seed, err := strconv.ParseUint(v, 10, 64)
@@ -946,7 +975,7 @@ func TestPowerLoss(t *testing.T) {
946 for i := range iterations {975 for i := range iterations {
947 seed := base + uint64(i)976 seed := base + uint64(i)
948 if !runPowerLoss(t, seed, m.opts) {977 if !runPowerLoss(t, seed, m.opts) {
949- t.Fatalf("failed with seed %d (ERMITE_KV_SEED=%d reproduces it)", seed, seed)978+ t.Fatalf("failed with seed %d (ERMITE_KV_SEED=%d replays its parameters)", seed, seed)
950 }979 }
951 }980 }
952 })981 })
M writer.go +3 -5
@@ -45,11 +45,9 @@ func (db *DB) writer() {
4545 defer func() {
4646 // A request taken but never grouped must still be answered.
4747 if pending != nil {
48- err := ErrClosed
49- select {
50- case <-db.failed:
51- err = db.failErr
52- default:
48+ err := db.stopped()
49+ if err == nil {
50+ err = ErrClosed // not reachable: the writer only exits on close or failure
5351 }
5452 pending.reply <- result{err: err}
5553 }
@@ -45,11 +45,9 @@ func (db *DB) writer() {
45 defer func() {45 defer func() {
46 // A request taken but never grouped must still be answered.46 // A request taken but never grouped must still be answered.
47 if pending != nil {47 if pending != nil {
48- err := ErrClosed48+ err := db.stopped()
49- select {49+ if err == nil {
50- case <-db.failed:50+ err = ErrClosed // not reachable: the writer only exits on close or failure
51- err = db.failErr
52- default:
53 }51 }
54 pending.reply <- result{err: err}52 pending.reply <- result{err: err}
55 }53 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 1

Tags 2

↑↓ moveenter openesc close