Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion internal/store/pebble.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ type PebbleStore struct {
// where the previous one stopped, wrapping at the end.
frontierCursorMu sync.Mutex
frontierCursor []byte

// PILOT-190: pebble.DB.Close() panics if called twice. Wrap teardown
// in sync.Once so repeated Close() calls (e.g. from layered cleanups
// or signal handlers) are idempotent. closeErr caches the first
// Close result so every caller sees the same outcome.
closeOnce sync.Once
closeErr error
}

const (
Expand Down Expand Up @@ -188,7 +195,16 @@ func OpenPebble(path string) (*PebbleStore, error) {
}

// Close flushes and closes the underlying Pebble DB.
func (p *PebbleStore) Close() error { return p.db.Close() }
//
// Idempotent: pebble.DB.Close() panics if called twice (PILOT-190), so
// the teardown is wrapped in sync.Once. Subsequent calls are no-ops and
// return the same error as the first call.
func (p *PebbleStore) Close() error {
p.closeOnce.Do(func() {
p.closeErr = p.db.Close()
})
return p.closeErr
}

// Checkpoint creates a consistent, point-in-time snapshot of the DB at destDir
// via hard-links to live SSTs. Cheap (no data copy) and safe to tar afterwards
Expand Down
29 changes: 29 additions & 0 deletions internal/store/pebble_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -661,3 +661,32 @@ func TestPebbleRecrawlURL(t *testing.T) {
t.Errorf("missing URL: want ErrNotFound, got %v", err)
}
}

// TestPebbleStore_CloseIsIdempotent — regression for PILOT-190.
// pebble.DB.Close() panics if called twice. PebbleStore wraps it in
// sync.Once so repeated Close() calls are safe and return the same
// error. Opens a store, calls Close() three times, asserts no panic
// and that every call returns the same value as the first.
func TestPebbleStore_CloseIsIdempotent(t *testing.T) {
// Skip newPebbleStore — its t.Cleanup() would also call Close()
// and we want to control invocation count explicitly.
dir := filepath.Join(t.TempDir(), "pebble")
p, err := OpenPebble(dir)
if err != nil {
t.Fatalf("OpenPebble: %v", err)
}

defer func() {
if r := recover(); r != nil {
t.Fatalf("Close() panicked on repeated invocation: %v", r)
}
}()

first := p.Close()
for i := 2; i <= 3; i++ {
got := p.Close()
if got != first {
t.Fatalf("Close() call #%d returned %v; want %v (cached first result)", i, got, first)
}
}
}
Loading