Skip to content

Commit

Permalink
session: fix the memory tracker leak in prepare binary protocol (#44618
Browse files Browse the repository at this point in the history
…) (#44639)

close #44612
  • Loading branch information
ti-chi-bot authored Jun 19, 2023
1 parent f917cc4 commit af1110c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
28 changes: 24 additions & 4 deletions server/conn_stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,21 +407,41 @@ func TestCursorDetachMemTracker(t *testing.T) {
// testkit also uses `PREPARE` related calls to run statement with arguments.
// format the SQL to avoid the interference from testkit.
tk.MustExec(fmt.Sprintf("set tidb_mem_quota_query=%d", maxConsumed/2))
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 1)
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 0)
// This query should exceed the memory limitation during `openExecutor`
require.Error(t, c.Dispatch(ctx, append(
appendUint32([]byte{mysql.ComStmtExecute}, uint32(stmt.ID())),
mysql.CursorTypeReadOnly, 0x1, 0x0, 0x0, 0x0,
)))
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 1)
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 0)

// The next query should succeed
tk.MustExec(fmt.Sprintf("set tidb_mem_quota_query=%d", maxConsumed+1))
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 1)
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 0)
// This query should succeed
require.NoError(t, c.Dispatch(ctx, append(
appendUint32([]byte{mysql.ComStmtExecute}, uint32(stmt.ID())),
mysql.CursorTypeReadOnly, 0x1, 0x0, 0x0, 0x0,
)))
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 1)
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 0)
}

func TestMemoryTrackForPrepareBinaryProtocol(t *testing.T) {
store, dom := testkit.CreateMockStoreAndDomain(t)
srv := CreateMockServer(t, store)
srv.SetDomain(dom)
defer srv.Close()

c := CreateMockConn(t, srv).(*mockConn)

tk := testkit.NewTestKitWithSession(t, store, c.Context().Session)
tk.MustExec("use test")
tk.MustExec("drop table if exists t")
tk.MustExec("create table t(id_2 int)")
for i := 0; i <= 10; i++ {
stmt, _, _, err := c.Context().Prepare("select count(id_2) from t")
require.NoError(t, err)
require.NoError(t, stmt.Close())
}
require.Len(t, tk.Session().GetSessionVars().MemTracker.GetChildrenForTest(), 0)
}
5 changes: 5 additions & 0 deletions session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -2447,6 +2447,11 @@ func (s *session) rollbackOnError(ctx context.Context) {

// PrepareStmt is used for executing prepare statement in binary protocol
func (s *session) PrepareStmt(sql string) (stmtID uint32, paramCount int, fields []*ast.ResultField, err error) {
defer func() {
if s.sessionVars.StmtCtx != nil {
s.sessionVars.StmtCtx.DetachMemDiskTracker()
}
}()
if s.sessionVars.TxnCtx.InfoSchema == nil {
// We don't need to create a transaction for prepare statement, just get information schema will do.
s.sessionVars.TxnCtx.InfoSchema = domain.GetDomain(s).InfoSchema()
Expand Down

0 comments on commit af1110c

Please sign in to comment.