Repository: polarsignals/frostdb (go)

Issue:
index: block rotation deadlock due to not releasing parts
Running 8c8df6673000 in a mirrored prod environment caused a deadlock to occur in frostdb. It seems like the issue is that queries are stuck collecting row groups because it acquires a read lock on the table block:
```
goroutine 42755 [sync.RWMutex.RLock, 3 minutes]:
sync.runtime_SemacquireRWMutexR(0xc0129f6f60?, 0xc0?, 0x4ce0640?)
    /Users/asubiotto/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22rc1.darwin-arm64/src/runtime/sema.go:82 +0x25
sync.(*RWMutex).RLock(...)
    /Users/asubiotto/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22rc1.darwin-arm64/src/sync/rwmutex.go:70
github.com/polarsignals/frostdb.(*Table).memoryBlocks(0xc000872280)
    /Users/asubiotto/go/pkg/mod/github.com/polarsignals/frostdb@v0.0.0-20240212201323-8c8df6673000/table.go:1177 +0x73
github.com/polarsignals/frostdb.(*Table).collectRowGroups(0xc000872280, {0x32a2cc0?, 0xc019fcc7d0?}, 0x2beff6, {0x32bcaa0, 0xc0129f6720}, 0x1, 0xc00ad7e4e0)
    /Users/asubiotto/go/pkg/mod/github.com/polarsignals/frostdb@v0.0.0-20240212201323-8c8df6673000/table.go:1214 +0x10f
github.com/polarsignals/frostdb.(*Table).Iterator.func3()
    /Users/asubiotto/go/pkg/mod/github.com/polarsignals/frostdb@v0.0.0-20240212201323-8c8df6673000/table.go:885 +0x77
golang.org/x/sync/errgroup.(*Group).Go.func1()
    /Users/asubiotto/go/pkg/mod/golang.org/x/sync@v0.5.0/errgroup/errgroup.go:75 +0x56
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 42738
    /Users/asubiotto/go/pkg/mod/golang.org/x/sync@v0.5.0/errgroup/errgroup.go:72 +0x96
```

The table block write lock is held by `dropPendingBlock` (called when rotating a block), which is stuck waiting for pending parts to be released:
```
goroutine 42697 [semacquire, 3 minutes]:
sync.runtime_Semacquire(0x9827f0?)
    /Users/asubiotto/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22rc1.darwin-arm64/src/runtime/sema.go:62 +0x25
sync.(*WaitGroup).Wait(0xc029c5ccc8?)
    /Users/asubiotto/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22rc1.darwin-arm64/src/sync/waitgroup.go:116 +0x48
github.com/polarsignals/frostdb/index.(*FileCompaction).Reset(0xc000ec58c0)
    /Users/asubiotto/go/pkg/mod/github.com/polarsignals/frostdb@v0.0.0-20240212201323-8c8df6673000/index/levels.go:179 +0x30
github.com/polarsignals/frostdb/index.(*LSM).Close(0xc000ede240)
    /Users/asubiotto/go/pkg/mod/github.com/polarsignals/frostdb@v0.0.0-20240212201323-8c8df6673000/index/lsm.go:192 +0x130
github.com/polarsignals/frostdb.(*Table).dropPendingBlock(0xc000872280, 0xc000e30990)
    /Users/asubiotto/go/pkg/mod/github.com/polarsignals/frostdb@v0.0.0-20240212201323-8c8df6673000/table.go:494 +0xd6
github.com/polarsignals/frostdb.(*Table).writeBlock(0xc000872280, 0xc000e30990, 0x0, 0x1)
    /Users/asubiotto/go/pkg/mod/github.com/polarsignals/frostdb@v0.0.0-20240212201323-8c8df6673000/table.go:512 +0x22e
created by github.com/polarsignals/frostdb.(*Table).RotateBlock in goroutine 42696
    /Users/asubiotto/go/pkg/mod/github.com/polarsignals/frostdb@v0.0.0-20240212201323-8c8df6673000/table.go:642 +0x4be
```

It seems likely that we're somehow not releasing parts correctly because there aren't any active compactions or recoveries (I searched for whether these types of stacktraces are stuck).


Discussion:

Patch:
diff --git a/table.go b/table.go
--- a/table.go
+++ b/table.go
@@ -1221,6 +1221,9 @@ func (t *Table) collectRowGroups(
 		if err := block.index.Scan(ctx, "", t.schema, filterExpr, tx, func(ctx context.Context, v any) error {
 			select {
 			case <-ctx.Done():
+				if rg, ok := v.(index.ReleaseableRowGroup); ok {
+					rg.Release()
+				}
 				return ctx.Err()
 			case rowGroups <- v:
 				return nil
@@ -1240,6 +1243,9 @@ func (t *Table) collectRowGroups(
 		if err := source.Scan(ctx, filepath.Join(t.db.name, t.name), t.schema, filterExpr, lastBlockTimestamp, func(ctx context.Context, v any) error {
 			select {
 			case <-ctx.Done():
+				if rg, ok := v.(index.ReleaseableRowGroup); ok {
+					rg.Release()
+				}
 				return ctx.Err()
 			case rowGroups <- v:
 				return nil


Test patch:
diff --git a/table_test.go b/table_test.go
--- a/table_test.go
+++ b/table_test.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"math"
 	"math/rand"
 	"runtime"
 	"sync"
@@ -1172,3 +1173,48 @@ func Test_Issue685(t *testing.T) {
 		})
 	require.NoError(t, err)
 }
+
+func Test_Issue741_Deadlock(t *testing.T) {
+	logger := newTestLogger(t)
+	dir := t.TempDir()
+	c, err := New(
+		WithIndexConfig([]*index.LevelConfig{
+			{Level: index.L0, MaxSize: 1 * TiB, Type: index.CompactionTypeParquetDisk},
+			{Level: index.L1, MaxSize: 1 * TiB},
+		}),
+		WithLogger(logger),
+		WithStoragePath(dir),
+		WithWAL(),
+		WithManualBlockRotation(),
+	)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		require.NoError(t, c.Close())
+	})
+
+	db, err := c.DB(context.Background(), "test")
+	require.NoError(t, err)
+
+	table, err := db.Table("test", NewTableConfig(dynparquet.SampleDefinition()))
+	require.NoError(t, err)
+
+	// Insert a record
+	r, err := dynparquet.GenerateTestSamples(1).ToRecord()
+	require.NoError(t, err)
+	ctx := context.Background()
+	_, err = table.InsertRecord(ctx, r)
+	require.NoError(t, err)
+
+	// Compact the table to create a Parquet file backed record
+	require.NoError(t, table.EnsureCompaction())
+
+	// Simulate a query that is canceled
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	require.Error(t, table.Iterator(ctx, math.MaxUint64, memory.NewGoAllocator(), []logicalplan.Callback{func(ctx context.Context, ar arrow.Record) error {
+		return nil
+	}}))
+
+	// This releases all the parts and waits for all reads to finish accessing the parts. This was causing a deadlock.
+	table.active.index.Close()
+}