From 19bd1da2236fac8e52c78cd247868805ce5eb16a Mon Sep 17 00:00:00 2001 From: Zhou Kunqin <25057648+time-and-fate@users.noreply.github.com> Date: Wed, 23 Aug 2023 13:09:34 +0800 Subject: [PATCH 1/2] This is an automated cherry-pick of #46201 Signed-off-by: ti-chi-bot --- executor/plan_replayer.go | 35 +++++++++++++++++++++++--- server/BUILD.bazel | 4 +++ server/server_test.go | 53 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/executor/plan_replayer.go b/executor/plan_replayer.go index f8a7ffe90fcca..f249256346c1e 100644 --- a/executor/plan_replayer.go +++ b/executor/plan_replayer.go @@ -60,12 +60,23 @@ type PlanReplayerCaptureInfo struct { // PlanReplayerDumpInfo indicates dump info type PlanReplayerDumpInfo struct { +<<<<<<< HEAD ExecStmts []ast.StmtNode Analyze bool Path string File *os.File FileName string ctx sessionctx.Context +======= + ExecStmts []ast.StmtNode + Analyze bool + HistoricalStatsTS uint64 + StartTS uint64 + Path string + File *os.File + FileName string + ctx sessionctx.Context +>>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) } // Next implements the Executor Next interface. @@ -84,6 +95,15 @@ func (e *PlanReplayerExec) Next(ctx context.Context, req *chunk.Chunk) error { if err != nil { return err } + // Note: + // For the dumping for SQL file case (len(e.DumpInfo.Path) > 0), the DumpInfo.dump() is called in + // handleFileTransInConn(), which is after TxnManager.OnTxnEnd(), where we can't access the TxnManager anymore. + // So we must fetch the startTS now. + startTS, err := sessiontxn.GetTxnManager(e.Ctx()).GetStmtReadTS() + if err != nil { + return err + } + e.DumpInfo.StartTS = startTS if len(e.DumpInfo.Path) > 0 { err = e.prepare() if err != nil { @@ -163,11 +183,8 @@ func (e *PlanReplayerExec) createFile() error { func (e *PlanReplayerDumpInfo) dump(ctx context.Context) (err error) { fileName := e.FileName zf := e.File - startTS, err := sessiontxn.GetTxnManager(e.ctx).GetStmtReadTS() - if err != nil { - return err - } task := &domain.PlanReplayerDumpTask{ +<<<<<<< HEAD StartTS: startTS, FileName: fileName, Zf: zf, @@ -175,6 +192,16 @@ func (e *PlanReplayerDumpInfo) dump(ctx context.Context) (err error) { TblStats: nil, ExecStmts: e.ExecStmts, Analyze: e.Analyze, +======= + StartTS: e.StartTS, + FileName: fileName, + Zf: zf, + SessionVars: e.ctx.GetSessionVars(), + TblStats: nil, + ExecStmts: e.ExecStmts, + Analyze: e.Analyze, + HistoricalStatsTS: e.HistoricalStatsTS, +>>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) } err = domain.DumpPlanReplayerInfo(ctx, e.ctx, task) if err != nil { diff --git a/server/BUILD.bazel b/server/BUILD.bazel index 10e2e927341e5..9171e7224a607 100644 --- a/server/BUILD.bazel +++ b/server/BUILD.bazel @@ -158,7 +158,11 @@ go_test( data = glob(["testdata/**"]), embed = [":server"], flaky = True, +<<<<<<< HEAD shard_count = 50, +======= + shard_count = 47, +>>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) deps = [ "//config", "//ddl", diff --git a/server/server_test.go b/server/server_test.go index 46efb5950037c..e60fcca450e20 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -34,10 +34,17 @@ import ( "github.com/go-sql-driver/mysql" "github.com/pingcap/errors" "github.com/pingcap/failpoint" +<<<<<<< HEAD "github.com/pingcap/log" "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/kv" tmysql "github.com/pingcap/tidb/parser/mysql" +======= + "github.com/pingcap/tidb/parser/mysql" + "github.com/pingcap/tidb/server/internal" + "github.com/pingcap/tidb/server/internal/testutil" + "github.com/pingcap/tidb/server/internal/util" +>>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) "github.com/pingcap/tidb/testkit" "github.com/pingcap/tidb/testkit/testenv" "github.com/pingcap/tidb/util/versioninfo" @@ -69,6 +76,7 @@ func newTestServerClient() *testServerClient { } } +<<<<<<< HEAD // statusURL return the full URL of a status path func (cli *testServerClient) statusURL(path string) string { return fmt.Sprintf("%s://localhost:%d%s", cli.statusScheme, cli.statusPort, path) @@ -2450,4 +2458,49 @@ func (cli *testServerClient) runTestInfoschemaClientErrors(t *testing.T) { } } }) +======= +func TestIssue46197(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tidbdrv := NewTiDBDriver(store) + cfg := util.NewTestConfig() + cfg.Port, cfg.Status.StatusPort = 0, 0 + cfg.Status.ReportStatus = false + server, err := NewServer(cfg, tidbdrv) + require.NoError(t, err) + defer server.Close() + + // Mock the content of the SQL file in PacketIO buffer. + // First 4 bytes are the header, followed by the actual content. + // This acts like we are sending "select * from t1;" from the client when tidb requests the "a.txt" file. + var inBuffer bytes.Buffer + _, err = inBuffer.Write([]byte{0x11, 0x00, 0x00, 0x01}) + require.NoError(t, err) + _, err = inBuffer.Write([]byte("select * from t1;")) + require.NoError(t, err) + + // clientConn setup + brc := util.NewBufferedReadConn(&testutil.BytesConn{Buffer: inBuffer}) + pkt := internal.NewPacketIO(brc) + pkt.SetBufWriter(bufio.NewWriter(bytes.NewBuffer(nil))) + cc := &clientConn{ + server: server, + alloc: arena.NewAllocator(1024), + chunkAlloc: chunk.NewAllocator(), + pkt: pkt, + capability: mysql.ClientLocalFiles, + } + ctx := context.Background() + cc.SetCtx(&TiDBContext{Session: tk.Session(), stmts: make(map[int]*TiDBStatement)}) + + tk.MustExec("use test") + tk.MustExec("create table t1 (a int, b int)") + + // 3 is mysql.ComQuery, followed by the SQL text. + require.NoError(t, cc.dispatch(ctx, []byte("\u0003plan replayer dump explain 'a.txt'"))) + + // clean up + path := testdata.ConvertRowsToStrings(tk.MustQuery("select @@tidb_last_plan_replayer_token").Rows()) + require.NoError(t, os.Remove(filepath.Join(replayer.GetPlanReplayerDirName(), path[0]))) +>>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) } From f9427536a82cee3ae4a0b3c3527072c7f77395a2 Mon Sep 17 00:00:00 2001 From: time-and-fate <25057648+time-and-fate@users.noreply.github.com> Date: Thu, 28 Sep 2023 16:30:09 +0800 Subject: [PATCH 2/2] resolve conflicts --- executor/plan_replayer.go | 27 +++------------------------ server/BUILD.bazel | 4 ---- server/server_test.go | 30 ++++++++++++++---------------- 3 files changed, 17 insertions(+), 44 deletions(-) diff --git a/executor/plan_replayer.go b/executor/plan_replayer.go index f249256346c1e..b8fc52e45071d 100644 --- a/executor/plan_replayer.go +++ b/executor/plan_replayer.go @@ -60,23 +60,13 @@ type PlanReplayerCaptureInfo struct { // PlanReplayerDumpInfo indicates dump info type PlanReplayerDumpInfo struct { -<<<<<<< HEAD ExecStmts []ast.StmtNode Analyze bool + StartTS uint64 Path string File *os.File FileName string ctx sessionctx.Context -======= - ExecStmts []ast.StmtNode - Analyze bool - HistoricalStatsTS uint64 - StartTS uint64 - Path string - File *os.File - FileName string - ctx sessionctx.Context ->>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) } // Next implements the Executor Next interface. @@ -99,7 +89,7 @@ func (e *PlanReplayerExec) Next(ctx context.Context, req *chunk.Chunk) error { // For the dumping for SQL file case (len(e.DumpInfo.Path) > 0), the DumpInfo.dump() is called in // handleFileTransInConn(), which is after TxnManager.OnTxnEnd(), where we can't access the TxnManager anymore. // So we must fetch the startTS now. - startTS, err := sessiontxn.GetTxnManager(e.Ctx()).GetStmtReadTS() + startTS, err := sessiontxn.GetTxnManager(e.ctx).GetStmtReadTS() if err != nil { return err } @@ -184,24 +174,13 @@ func (e *PlanReplayerDumpInfo) dump(ctx context.Context) (err error) { fileName := e.FileName zf := e.File task := &domain.PlanReplayerDumpTask{ -<<<<<<< HEAD - StartTS: startTS, + StartTS: e.StartTS, FileName: fileName, Zf: zf, SessionVars: e.ctx.GetSessionVars(), TblStats: nil, ExecStmts: e.ExecStmts, Analyze: e.Analyze, -======= - StartTS: e.StartTS, - FileName: fileName, - Zf: zf, - SessionVars: e.ctx.GetSessionVars(), - TblStats: nil, - ExecStmts: e.ExecStmts, - Analyze: e.Analyze, - HistoricalStatsTS: e.HistoricalStatsTS, ->>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) } err = domain.DumpPlanReplayerInfo(ctx, e.ctx, task) if err != nil { diff --git a/server/BUILD.bazel b/server/BUILD.bazel index 9171e7224a607..10e2e927341e5 100644 --- a/server/BUILD.bazel +++ b/server/BUILD.bazel @@ -158,11 +158,7 @@ go_test( data = glob(["testdata/**"]), embed = [":server"], flaky = True, -<<<<<<< HEAD shard_count = 50, -======= - shard_count = 47, ->>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) deps = [ "//config", "//ddl", diff --git a/server/server_test.go b/server/server_test.go index e60fcca450e20..39bea4cadc40d 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -15,7 +15,9 @@ package server import ( + "bufio" "bytes" + "context" "database/sql" "encoding/json" "fmt" @@ -34,19 +36,16 @@ import ( "github.com/go-sql-driver/mysql" "github.com/pingcap/errors" "github.com/pingcap/failpoint" -<<<<<<< HEAD "github.com/pingcap/log" "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/kv" tmysql "github.com/pingcap/tidb/parser/mysql" -======= - "github.com/pingcap/tidb/parser/mysql" - "github.com/pingcap/tidb/server/internal" - "github.com/pingcap/tidb/server/internal/testutil" - "github.com/pingcap/tidb/server/internal/util" ->>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) "github.com/pingcap/tidb/testkit" + "github.com/pingcap/tidb/testkit/testdata" "github.com/pingcap/tidb/testkit/testenv" + "github.com/pingcap/tidb/util/arena" + "github.com/pingcap/tidb/util/chunk" + "github.com/pingcap/tidb/util/replayer" "github.com/pingcap/tidb/util/versioninfo" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -76,7 +75,6 @@ func newTestServerClient() *testServerClient { } } -<<<<<<< HEAD // statusURL return the full URL of a status path func (cli *testServerClient) statusURL(path string) string { return fmt.Sprintf("%s://localhost:%d%s", cli.statusScheme, cli.statusPort, path) @@ -2458,12 +2456,13 @@ func (cli *testServerClient) runTestInfoschemaClientErrors(t *testing.T) { } } }) -======= +} + func TestIssue46197(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) tidbdrv := NewTiDBDriver(store) - cfg := util.NewTestConfig() + cfg := newTestConfig() cfg.Port, cfg.Status.StatusPort = 0, 0 cfg.Status.ReportStatus = false server, err := NewServer(cfg, tidbdrv) @@ -2480,18 +2479,18 @@ func TestIssue46197(t *testing.T) { require.NoError(t, err) // clientConn setup - brc := util.NewBufferedReadConn(&testutil.BytesConn{Buffer: inBuffer}) - pkt := internal.NewPacketIO(brc) - pkt.SetBufWriter(bufio.NewWriter(bytes.NewBuffer(nil))) + brc := newBufferedReadConn(&bytesConn{b: inBuffer}) + pkt := newPacketIO(brc) + pkt.bufWriter = bufio.NewWriter(bytes.NewBuffer(nil)) cc := &clientConn{ server: server, alloc: arena.NewAllocator(1024), chunkAlloc: chunk.NewAllocator(), pkt: pkt, - capability: mysql.ClientLocalFiles, + capability: tmysql.ClientLocalFiles, } ctx := context.Background() - cc.SetCtx(&TiDBContext{Session: tk.Session(), stmts: make(map[int]*TiDBStatement)}) + cc.setCtx(&TiDBContext{Session: tk.Session(), stmts: make(map[int]*TiDBStatement)}) tk.MustExec("use test") tk.MustExec("create table t1 (a int, b int)") @@ -2502,5 +2501,4 @@ func TestIssue46197(t *testing.T) { // clean up path := testdata.ConvertRowsToStrings(tk.MustQuery("select @@tidb_last_plan_replayer_token").Rows()) require.NoError(t, os.Remove(filepath.Join(replayer.GetPlanReplayerDirName(), path[0]))) ->>>>>>> 9466e45f7d0 (executor: fix plan replayer for sql file input wrongly fetch startTS after `OnTxnEnd` (#46201)) }