Skip to content

Commit f2194a9

Browse files
jbowensjbowens
committed
wal: clean up tests, fix leaked goroutine
The TestFailoverManager_AllFilesDeletable unit test was leaking a goroutine through failing to Close the FailoverManager. This commit fixes the test to Close the manager. It also adds leaktest across the package's unit tests and adapts one test to use synctest.Test to reduce its execution time.
1 parent 8dfb102 commit f2194a9

File tree

3 files changed

+56
-37
lines changed

3 files changed

+56
-37
lines changed

‎wal/failover_manager_test.go‎

Lines changed: 41 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ import (
1414
"strings"
1515
"sync"
1616
"testing"
17+
"testing/synctest"
1718
"time"
1819

20+
"github.com/cockroachdb/crlib/testutils/leaktest"
1921
"github.com/cockroachdb/datadriven"
2022
"github.com/cockroachdb/pebble/internal/testutils"
2123
"github.com/cockroachdb/pebble/vfs"
@@ -572,41 +574,45 @@ func recvWithDeadline(t *testing.T, td *datadriven.TestData, waitStr string, ch
572574
}
573575

574576
func TestFailoverManager_Quiesce(t *testing.T) {
575-
seed := time.Now().UnixNano()
576-
memFS := vfs.NewMem()
577-
require.NoError(t, memFS.MkdirAll("primary", os.ModePerm))
578-
require.NoError(t, memFS.MkdirAll("secondary", os.ModePerm))
579-
fs := errorfs.Wrap(memFS, errorfs.RandomLatency(
580-
errorfs.Randomly(0.50, seed), 10*time.Millisecond, seed, 0 /* no limit */))
581-
582-
var m failoverManager
583-
require.NoError(t, m.init(Options{
584-
Primary: Dir{FS: fs, Dirname: "primary"},
585-
Secondary: Dir{FS: fs, Dirname: "secondary"},
586-
MaxNumRecyclableLogs: 2,
587-
PreallocateSize: func() int { return 4 },
588-
FailoverOptions: FailoverOptions{
589-
PrimaryDirProbeInterval: 250 * time.Microsecond,
590-
HealthyProbeLatencyThreshold: time.Millisecond,
591-
HealthyInterval: 3 * time.Millisecond,
592-
UnhealthySamplingInterval: 250 * time.Microsecond,
593-
UnhealthyOperationLatencyThreshold: func() (time.Duration, bool) { return time.Millisecond, true },
594-
},
595-
FailoverWriteAndSyncLatency: prometheus.NewHistogram(prometheus.HistogramOpts{}),
596-
WriteWALSyncOffsets: func() bool { return false },
597-
}, nil /* initial logs */))
598-
for i := 0; i < 3; i++ {
599-
w, err := m.Create(NumWAL(i), i)
600-
require.NoError(t, err)
601-
_, err = w.WriteRecord([]byte("hello world"), SyncOptions{}, nil)
602-
require.NoError(t, err)
603-
_, err = w.Close()
604-
require.NoError(t, err)
605-
}
606-
require.NoError(t, m.Close())
577+
synctest.Test(t, func(t *testing.T) {
578+
seed := time.Now().UnixNano()
579+
memFS := vfs.NewMem()
580+
require.NoError(t, memFS.MkdirAll("primary", os.ModePerm))
581+
require.NoError(t, memFS.MkdirAll("secondary", os.ModePerm))
582+
fs := errorfs.Wrap(memFS, errorfs.RandomLatency(
583+
errorfs.Randomly(0.50, seed), 10*time.Millisecond, seed, 0 /* no limit */))
584+
585+
var m failoverManager
586+
require.NoError(t, m.init(Options{
587+
Primary: Dir{FS: fs, Dirname: "primary"},
588+
Secondary: Dir{FS: fs, Dirname: "secondary"},
589+
MaxNumRecyclableLogs: 2,
590+
PreallocateSize: func() int { return 4 },
591+
FailoverOptions: FailoverOptions{
592+
PrimaryDirProbeInterval: 250 * time.Microsecond,
593+
HealthyProbeLatencyThreshold: time.Millisecond,
594+
HealthyInterval: 3 * time.Millisecond,
595+
UnhealthySamplingInterval: 250 * time.Microsecond,
596+
UnhealthyOperationLatencyThreshold: func() (time.Duration, bool) { return time.Millisecond, true },
597+
},
598+
FailoverWriteAndSyncLatency: prometheus.NewHistogram(prometheus.HistogramOpts{}),
599+
WriteWALSyncOffsets: func() bool { return false },
600+
}, nil /* initial logs */))
601+
for i := range 3 {
602+
w, err := m.Create(NumWAL(i), i)
603+
require.NoError(t, err)
604+
_, err = w.WriteRecord([]byte("hello world"), SyncOptions{}, nil)
605+
require.NoError(t, err)
606+
_, err = w.Close()
607+
require.NoError(t, err)
608+
}
609+
require.NoError(t, m.Close())
610+
})
607611
}
608612

609613
func TestFailoverManager_SecondaryIsWritable(t *testing.T) {
614+
defer leaktest.AfterTest(t)()
615+
610616
var m failoverManager
611617
require.EqualError(t, m.init(Options{
612618
Primary: Dir{FS: vfs.NewMem(), Dirname: "primary"},
@@ -627,6 +633,8 @@ func TestFailoverManager_SecondaryIsWritable(t *testing.T) {
627633
// that all the files that are created by the manager are eventually returned by
628634
// FailoverManager.Obsolete for deletion.
629635
func TestFailoverManager_AllFilesDeletable(t *testing.T) {
636+
defer leaktest.AfterTest(t)()
637+
630638
seed := time.Now().UnixNano()
631639
memFS := vfs.NewMem()
632640
require.NoError(t, memFS.MkdirAll("primary", os.ModePerm))
@@ -638,6 +646,7 @@ func TestFailoverManager_AllFilesDeletable(t *testing.T) {
638646
errorfs.Randomly(0.50, latencySeed), 10*time.Millisecond, latencySeed, 0 /* no limit */))
639647

640648
var m failoverManager
649+
defer func() { require.NoError(t, m.Close()) }()
641650
require.NoError(t, m.init(Options{
642651
Primary: Dir{FS: fs, Dirname: "primary"},
643652
Secondary: Dir{FS: fs, Dirname: "secondary"},

‎wal/failover_writer_test.go‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"testing"
1717
"time"
1818

19+
"github.com/cockroachdb/crlib/testutils/leaktest"
1920
"github.com/cockroachdb/datadriven"
2021
"github.com/cockroachdb/errors"
2122
"github.com/cockroachdb/pebble/internal/base"
@@ -37,6 +38,8 @@ const (
3738
)
3839

3940
func TestFailoverWriter(t *testing.T) {
41+
defer leaktest.AfterTest(t)()
42+
4043
datadriven.Walk(t, "testdata/failover_writer", func(t *testing.T, path string) {
4144
memFS := vfs.NewCrashableMem()
4245
dirs := [numDirIndices]dirAndFileHandle{
@@ -417,7 +420,7 @@ func TestFailoverWriter(t *testing.T) {
417420
return "no ongoing"
418421
}
419422
// Timeout eventually, if the state is unexpected.
420-
for i := 0; i < 4000; i++ {
423+
for range 4000 {
421424
d, _ = w.mu.writers[index].r.ongoingLatencyOrError()
422425
if (d > 0) == expectedOngoing {
423426
return returnStr()
@@ -598,6 +601,8 @@ func (f blockingFile) Close() error {
598601
// with CAS failure, and resulting retries. (c) is observable in this test by
599602
// adding print statements in recordQueue.pop.
600603
func TestConcurrentWritersWithManyRecords(t *testing.T) {
604+
defer leaktest.AfterTest(t)()
605+
601606
seed := *seed
602607
if seed == 0 {
603608
seed = time.Now().UnixNano()
@@ -673,14 +678,14 @@ func TestConcurrentWritersWithManyRecords(t *testing.T) {
673678
}
674679
}
675680
time.Sleep(5 * time.Millisecond)
676-
for i := 0; i < numLogWriters; i++ {
681+
for i := range numLogWriters {
677682
bFS.setConf(makeLogFilename(0, LogNameIndex(i)), 0)
678683
}
679684
_, err = ww.Close()
680685
require.NoError(t, err)
681686
wg.Wait()
682687
func() {
683-
for i := 0; i < 100; i++ {
688+
for range 100 {
684689
if len(queueSemChan) == 0 {
685690
return
686691
}
@@ -691,7 +696,7 @@ func TestConcurrentWritersWithManyRecords(t *testing.T) {
691696
type indexInterval struct {
692697
first, last int
693698
}
694-
for i := 0; i < numLogWriters; i++ {
699+
for i := range numLogWriters {
695700
func() {
696701
f, err := memFS.Open(memFS.PathJoin(dirs[i%2].Dirname, makeLogFilename(0, LogNameIndex(i))))
697702
if err != nil {
@@ -739,6 +744,8 @@ func randStr(fill []byte, rng *rand.Rand) {
739744
}
740745

741746
func TestFailoverWriterManyRecords(t *testing.T) {
747+
defer leaktest.AfterTest(t)()
748+
742749
stopper := newStopper()
743750
memFS := vfs.NewMem()
744751
f, err := memFS.OpenDir("")
@@ -763,7 +770,7 @@ func TestFailoverWriterManyRecords(t *testing.T) {
763770
const count = 4 * initialBufferLen
764771
wg := &sync.WaitGroup{}
765772
wg.Add(count)
766-
for i := 0; i < count; i++ {
773+
for range count {
767774
_, err := w.WriteRecord(buf[:], SyncOptions{Done: wg, Err: new(error)}, nil)
768775
require.NoError(t, err)
769776
}

‎wal/reader_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"testing"
1818
"unicode"
1919

20+
"github.com/cockroachdb/crlib/testutils/leaktest"
2021
"github.com/cockroachdb/datadriven"
2122
"github.com/cockroachdb/pebble/batchrepr"
2223
"github.com/cockroachdb/pebble/internal/base"
@@ -98,6 +99,8 @@ func TestList(t *testing.T) {
9899
// TestReader tests the virtual WAL reader that merges across multiple physical
99100
// log files.
100101
func TestReader(t *testing.T) {
102+
defer leaktest.AfterTest(t)()
103+
101104
rng := rand.New(rand.NewPCG(1, 1))
102105
var buf bytes.Buffer
103106
var fs vfs.FS

0 commit comments

Comments
 (0)