diff --git a/CHANGELOG.md b/CHANGELOG.md index bf57c4be05..50276edb9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ ## master / unreleased + - [BUGFIX] Calling `Close` more than once on a querier returns an error instead of a panic. + ## 0.7.1 - [ENHANCEMENT] Reduce memory usage in mergedPostings.Seek diff --git a/querier.go b/querier.go index 3ec2ff3dc3..b7116c27e2 100644 --- a/querier.go +++ b/querier.go @@ -205,6 +205,8 @@ type blockQuerier struct { chunks ChunkReader tombstones TombstoneReader + closed bool + mint, maxt int64 } @@ -252,12 +254,15 @@ func (q *blockQuerier) LabelValuesFor(string, labels.Label) ([]string, error) { } func (q *blockQuerier) Close() error { - var merr tsdb_errors.MultiError + if q.closed { + return errors.New("block querier already closed") + } + var merr tsdb_errors.MultiError merr.Add(q.index.Close()) merr.Add(q.chunks.Close()) merr.Add(q.tombstones.Close()) - + q.closed = true return merr.Err() } diff --git a/querier_test.go b/querier_test.go index d190840fd8..cb53462a13 100644 --- a/querier_test.go +++ b/querier_test.go @@ -1898,3 +1898,30 @@ func TestPostingsForMatchers(t *testing.T) { } } + +// TestClose ensures that calling Close more than once doesn't block and doesn't panic. +func TestClose(t *testing.T) { + dir, err := ioutil.TempDir("", "test_storage") + if err != nil { + t.Fatalf("Opening test dir failed: %s", err) + } + defer func() { + testutil.Ok(t, os.RemoveAll(dir)) + }() + + createBlock(t, dir, genSeries(1, 1, 0, 10)) + createBlock(t, dir, genSeries(1, 1, 10, 20)) + + db, err := Open(dir, nil, nil, DefaultOptions) + if err != nil { + t.Fatalf("Opening test storage failed: %s", err) + } + defer func() { + testutil.Ok(t, db.Close()) + }() + + q, err := db.Querier(0, 20) + testutil.Ok(t, err) + testutil.Ok(t, q.Close()) + testutil.NotOk(t, q.Close()) +} diff --git a/wal/wal.go b/wal/wal.go index 0001147a75..a51d43614e 100644 --- a/wal/wal.go +++ b/wal/wal.go @@ -159,9 +159,9 @@ type WAL struct { logger log.Logger segmentSize int mtx sync.RWMutex - segment *Segment // active segment - donePages int // pages written to the segment - page *page // active page + segment *Segment // Active segment. + donePages int // Pages written to the segment. + page *page // Active page. stopc chan chan struct{} actorc chan func() closed bool // To allow calling Close() more than once without blocking. @@ -606,7 +606,7 @@ func (w *WAL) Close() (err error) { defer w.mtx.Unlock() if w.closed { - return nil + return errors.New("wal already closed") } // Flush the last page and zero out all its remaining size. diff --git a/wal/wal_test.go b/wal/wal_test.go index e7b893a36f..391c4b6567 100644 --- a/wal/wal_test.go +++ b/wal/wal_test.go @@ -305,6 +305,19 @@ func TestCorruptAndCarryOn(t *testing.T) { } } +// TestClose ensures that calling Close more than once doesn't panic and doesn't block. +func TestClose(t *testing.T) { + dir, err := ioutil.TempDir("", "wal_repair") + testutil.Ok(t, err) + defer func() { + testutil.Ok(t, os.RemoveAll(dir)) + }() + w, err := NewSize(nil, nil, dir, pageSize) + testutil.Ok(t, err) + testutil.Ok(t, w.Close()) + testutil.NotOk(t, w.Close()) +} + func BenchmarkWAL_LogBatched(b *testing.B) { dir, err := ioutil.TempDir("", "bench_logbatch") testutil.Ok(b, err)