Skip to content

Commit 07ae424

Browse files
committed
clean rowContainer even if the execution paniced
Signed-off-by: Yang Keao <[email protected]>
1 parent ae830dc commit 07ae424

File tree

3 files changed

+62
-6
lines changed

3 files changed

+62
-6
lines changed

pkg/server/conn_stmt.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,14 @@ func (cc *clientConn) executeWithCursor(ctx context.Context, stmt PreparedStatem
385385
action := memory.NewActionWithPriority(rowContainer.ActionSpill(), memory.DefCursorFetchSpillPriority)
386386
vars.MemTracker.FallbackOldAndSetNewAction(action)
387387
}
388+
// store the rowContainer in the statement right after it's created, so that even if the logic in defer is not triggered,
389+
// the rowContainer will be released when the statement is closed.
390+
stmt.StoreRowContainer(rowContainer)
388391
defer func() {
389392
if err != nil {
393+
// if the execution panic, it'll not reach this branch. The `rowContainer` will be released in the `stmt.Close`.
394+
stmt.StoreRowContainer(nil)
395+
390396
rowContainer.GetMemTracker().Detach()
391397
rowContainer.GetDiskTracker().Detach()
392398
errCloseRowContainer := rowContainer.Close()
@@ -424,7 +430,6 @@ func (cc *clientConn) executeWithCursor(ctx context.Context, stmt PreparedStatem
424430
if cl, ok := crs.(resultset.FetchNotifier); ok {
425431
cl.OnFetchReturned()
426432
}
427-
stmt.StoreRowContainer(rowContainer)
428433

429434
err = cc.writeExecuteResultWithCursor(ctx, stmt, crs)
430435
return false, err
@@ -448,16 +453,13 @@ func (cc *clientConn) executeWithLazyCursor(ctx context.Context, stmt PreparedSt
448453

449454
// writeExecuteResultWithCursor will store the `ResultSet` in `stmt` and send the column info to the client. The logic is shared between
450455
// lazy cursor fetch and normal(eager) cursor fetch.
451-
func (cc *clientConn) writeExecuteResultWithCursor(ctx context.Context, stmt PreparedStatement, rs resultset.CursorResultSet) error {
452-
var err error
453-
456+
func (cc *clientConn) writeExecuteResultWithCursor(ctx context.Context, stmt PreparedStatement, rs resultset.CursorResultSet) (err error) {
454457
stmt.StoreResultSet(rs)
455458
stmt.SetCursorActive(true)
456459
defer func() {
457460
if err != nil {
458461
// the resultSet and rowContainer have been closed in former "defer" statement.
459462
stmt.StoreResultSet(nil)
460-
stmt.StoreRowContainer(nil)
461463
stmt.SetCursorActive(false)
462464
}
463465
}()

pkg/server/tests/cursor/BUILD.bazel

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@ go_test(
88
"main_test.go",
99
],
1010
flaky = True,
11-
shard_count = 8,
11+
shard_count = 9,
1212
deps = [
1313
"//pkg/config",
14+
"//pkg/executor",
1415
"//pkg/metrics",
1516
"//pkg/parser/mysql",
1617
"//pkg/server",
18+
"//pkg/server/internal/util",
1719
"//pkg/server/tests/servertestkit",
1820
"//pkg/store/mockstore/unistore",
1921
"//pkg/testkit",

pkg/server/tests/cursor/cursor_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@ import (
2121
"fmt"
2222
"io"
2323
"math/rand"
24+
"os"
2425
"testing"
26+
"time"
2527

2628
mysqlcursor "github.com/YangKeao/go-mysql-driver"
2729
"github.com/pingcap/failpoint"
2830
"github.com/pingcap/tidb/pkg/config"
31+
"github.com/pingcap/tidb/pkg/executor"
2932
tmysql "github.com/pingcap/tidb/pkg/parser/mysql"
3033
server2 "github.com/pingcap/tidb/pkg/server"
34+
util2 "github.com/pingcap/tidb/pkg/server/internal/util"
3135
"github.com/pingcap/tidb/pkg/server/tests/servertestkit"
3236
"github.com/pingcap/tidb/pkg/testkit"
3337
"github.com/stretchr/testify/require"
@@ -506,3 +510,51 @@ outerLoop:
506510
}
507511
}
508512
}
513+
514+
func TestCursorExceedQuota(t *testing.T) {
515+
cfg := util2.NewTestConfig()
516+
cfg.TempStoragePath = t.TempDir()
517+
518+
cfg.Port = 0
519+
cfg.Status.StatusPort = 0
520+
cfg.TempStorageQuota = 1000
521+
executor.GlobalDiskUsageTracker.SetBytesLimit(cfg.TempStorageQuota)
522+
ts := servertestkit.CreateTidbTestSuiteWithCfg(t, cfg)
523+
524+
mysqldriver := &mysqlcursor.MySQLDriver{}
525+
rawConn, err := mysqldriver.Open(ts.GetDSNWithCursor(10))
526+
require.NoError(t, err)
527+
conn := rawConn.(mysqlcursor.Connection)
528+
529+
_, err = conn.ExecContext(context.Background(), "drop table if exists t1", nil)
530+
require.NoError(t, err)
531+
_, err = conn.ExecContext(context.Background(), "CREATE TABLE `t1` (`c1` varchar(100));", nil)
532+
require.NoError(t, err)
533+
rowCount := 1000
534+
for i := 0; i < rowCount; i++ {
535+
_, err = conn.ExecContext(context.Background(), "insert into t1 (c1) values ('201801');", nil)
536+
require.NoError(t, err)
537+
}
538+
539+
_, err = conn.ExecContext(context.Background(), "set tidb_mem_quota_query = 1;", nil)
540+
require.NoError(t, err)
541+
_, err = conn.ExecContext(context.Background(), "set global tidb_enable_tmp_storage_on_oom = 'ON'", nil)
542+
require.NoError(t, err)
543+
544+
rawStmt, err := conn.Prepare("SELECT * FROM test.t1")
545+
require.NoError(t, err)
546+
stmt := rawStmt.(mysqlcursor.Statement)
547+
548+
_, err = stmt.QueryContext(context.Background(), nil)
549+
require.Error(t, err)
550+
require.Contains(t, err.Error(), "Out Of Quota For Local Temporary Space!")
551+
552+
require.NoError(t, conn.Close())
553+
554+
time.Sleep(time.Second)
555+
556+
tempStoragePath := cfg.TempStoragePath
557+
files, err := os.ReadDir(tempStoragePath)
558+
require.NoError(t, err)
559+
require.Empty(t, files)
560+
}

0 commit comments

Comments
 (0)