ddl: refine GetDDLInfo function (#35969)

pingcap · Jul 6, 2022 · 0f7faa6 · 0f7faa6
1 parent 2df6be5
commit 0f7faa6
Show file tree

Hide file tree

Showing 6 changed files with 94 additions and 71 deletions.
diff --git a/ddl/ddl.go b/ddl/ddl.go
@@ -977,10 +977,25 @@ type Info struct {
 	Jobs        []*model.Job // It's the currently running jobs.
 }
 
+// GetDDLInfoWithNewTxn returns DDL information using a new txn.
+func GetDDLInfoWithNewTxn(s sessionctx.Context) (*Info, error) {
+	err := sessiontxn.NewTxn(context.Background(), s)
+	if err != nil {
+		return nil, err
+	}
+	info, err := GetDDLInfo(s)
+	s.RollbackTxn(context.Background())
+	return info, err
+}
+
 // GetDDLInfo returns DDL information.
-func GetDDLInfo(txn kv.Transaction) (*Info, error) {
+func GetDDLInfo(s sessionctx.Context) (*Info, error) {
 	var err error
 	info := &Info{}
+	txn, err := s.Txn(true)
+	if err != nil {
+		return nil, err
+	}
 	t := meta.NewMeta(txn)
 
 	info.Jobs = make([]*model.Job, 0, 2)

diff --git a/ddl/ddl_test.go b/ddl/ddl_test.go
@@ -689,55 +689,6 @@ func TestReorg(t *testing.T) {
 	}
 }
 
-func TestGetDDLInfo(t *testing.T) {
-	store, clean := newMockStore(t)
-	defer clean()
-
-	txn, err := store.Begin()
-	require.NoError(t, err)
-	m := meta.NewMeta(txn)
-
-	dbInfo2 := &model.DBInfo{
-		ID:    2,
-		Name:  model.NewCIStr("b"),
-		State: model.StateNone,
-	}
-	job := &model.Job{
-		SchemaID: dbInfo2.ID,
-		Type:     model.ActionCreateSchema,
-		RowCount: 0,
-	}
-	job1 := &model.Job{
-		SchemaID: dbInfo2.ID,
-		Type:     model.ActionAddIndex,
-		RowCount: 0,
-	}
-
-	err = m.EnQueueDDLJob(job)
-	require.NoError(t, err)
-
-	info, err := GetDDLInfo(txn)
-	require.NoError(t, err)
-	require.Len(t, info.Jobs, 1)
-	require.Equal(t, job, info.Jobs[0])
-	require.Nil(t, info.ReorgHandle)
-
-	// two jobs
-	m = meta.NewMeta(txn, meta.AddIndexJobListKey)
-	err = m.EnQueueDDLJob(job1)
-	require.NoError(t, err)
-
-	info, err = GetDDLInfo(txn)
-	require.NoError(t, err)
-	require.Len(t, info.Jobs, 2)
-	require.Equal(t, job, info.Jobs[0])
-	require.Equal(t, job1, info.Jobs[1])
-	require.Nil(t, info.ReorgHandle)
-
-	err = txn.Rollback()
-	require.NoError(t, err)
-}
-
 func TestGetDDLJobs(t *testing.T) {
 	store, clean := newMockStore(t)
 	defer clean()

diff --git a/ddl/stat.go b/ddl/stat.go
@@ -15,10 +15,7 @@
 package ddl
 
 import (
-	"context"
-
 	"github.com/pingcap/errors"
-	"github.com/pingcap/tidb/kv"
 	"github.com/pingcap/tidb/sessionctx/variable"
 )
 
@@ -51,15 +48,12 @@ func (d *ddl) Stats(vars *variable.SessionVars) (map[string]interface{}, error)
 	m[serverID] = d.uuid
 	var ddlInfo *Info
 
-	ctx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL)
-	err := kv.RunInNewTxn(ctx, d.store, false, func(ctx context.Context, txn kv.Transaction) error {
-		var err1 error
-		ddlInfo, err1 = GetDDLInfo(txn)
-		if err1 != nil {
-			return errors.Trace(err1)
-		}
-		return errors.Trace(err1)
-	})
+	s, err := d.sessPool.get()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	defer d.sessPool.put(s)
+	ddlInfo, err = GetDDLInfoWithNewTxn(s)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}

diff --git a/ddl/stat_test.go b/ddl/stat_test.go
@@ -20,6 +20,8 @@ import (
 
 	"github.com/pingcap/failpoint"
 	"github.com/pingcap/tidb/ddl"
+	"github.com/pingcap/tidb/kv"
+	"github.com/pingcap/tidb/meta"
 	"github.com/pingcap/tidb/parser/ast"
 	"github.com/pingcap/tidb/parser/model"
 	"github.com/pingcap/tidb/sessionctx"
@@ -86,6 +88,66 @@ func TestDDLStatsInfo(t *testing.T) {
 	}
 }
 
+func TestGetDDLInfo(t *testing.T) {
+	store, clean := testkit.CreateMockStore(t)
+	defer clean()
+
+	sess := testkit.NewTestKit(t, store).Session()
+	_, err := sess.Execute(context.Background(), "begin")
+	require.NoError(t, err)
+	txn, err := sess.Txn(true)
+	require.NoError(t, err)
+
+	dbInfo2 := &model.DBInfo{
+		ID:    2,
+		Name:  model.NewCIStr("b"),
+		State: model.StateNone,
+	}
+	job := &model.Job{
+		ID:       1,
+		SchemaID: dbInfo2.ID,
+		Type:     model.ActionCreateSchema,
+		RowCount: 0,
+	}
+	job1 := &model.Job{
+		ID:       2,
+		SchemaID: dbInfo2.ID,
+		Type:     model.ActionAddIndex,
+		RowCount: 0,
+	}
+
+	err = addDDLJobs(txn, job)
+	require.NoError(t, err)
+
+	info, err := ddl.GetDDLInfo(sess)
+	require.NoError(t, err)
+	require.Len(t, info.Jobs, 1)
+	require.Equal(t, job, info.Jobs[0])
+	require.Nil(t, info.ReorgHandle)
+
+	// two jobs
+	err = addDDLJobs(txn, job1)
+	require.NoError(t, err)
+
+	info, err = ddl.GetDDLInfo(sess)
+	require.NoError(t, err)
+	require.Len(t, info.Jobs, 2)
+	require.Equal(t, job, info.Jobs[0])
+	require.Equal(t, job1, info.Jobs[1])
+	require.Nil(t, info.ReorgHandle)
+
+	_, err = sess.Execute(context.Background(), "rollback")
+	require.NoError(t, err)
+}
+
+func addDDLJobs(txn kv.Transaction, job *model.Job) error {
+	m := meta.NewMeta(txn)
+	if job.MayNeedReorg() {
+		return m.EnQueueDDLJob(job, meta.AddIndexJobListKey)
+	}
+	return m.EnQueueDDLJob(job)
+}
+
 func buildCreateIdxJob(dbInfo *model.DBInfo, tblInfo *model.TableInfo, unique bool, indexName string, colName string) *model.Job {
 	return &model.Job{
 		SchemaID:   dbInfo.ID,

diff --git a/executor/builder.go b/executor/builder.go
@@ -351,13 +351,14 @@ func (b *executorBuilder) buildShowDDL(v *plannercore.ShowDDL) Executor {
 		b.err = err
 		return nil
 	}
-	txn, err := e.ctx.Txn(true)
+
+	session, err := e.getSysSession()
 	if err != nil {
 		b.err = err
 		return nil
 	}
-
-	ddlInfo, err := ddl.GetDDLInfo(txn)
+	ddlInfo, err := ddl.GetDDLInfoWithNewTxn(session)
+	e.releaseSysSession(kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL), session)
 	if err != nil {
 		b.err = err
 		return nil

diff --git a/executor/executor_test.go b/executor/executor_test.go
@@ -5623,9 +5623,10 @@ func TestAdmin(t *testing.T) {
 	require.NoError(t, err)
 	row = req.GetRow(0)
 	require.Equal(t, 6, row.Len())
-	txn, err := store.Begin()
-	require.NoError(t, err)
-	ddlInfo, err := ddl.GetDDLInfo(txn)
+	tk = testkit.NewTestKit(t, store)
+	tk.MustExec("begin")
+	sess := tk.Session()
+	ddlInfo, err := ddl.GetDDLInfo(sess)
 	require.NoError(t, err)
 	require.Equal(t, ddlInfo.SchemaVer, row.GetInt64(0))
 	// TODO: Pass this test.
@@ -5640,8 +5641,7 @@ func TestAdmin(t *testing.T) {
 	err = r.Next(ctx, req)
 	require.NoError(t, err)
 	require.Zero(t, req.NumRows())
-	err = txn.Rollback()
-	require.NoError(t, err)
+	tk.MustExec("rollback")
 
 	// show DDL jobs test
 	r, err = tk.Exec("admin show ddl jobs")
@@ -5651,7 +5651,7 @@ func TestAdmin(t *testing.T) {
 	require.NoError(t, err)
 	row = req.GetRow(0)
 	require.Equal(t, 12, row.Len())
-	txn, err = store.Begin()
+	txn, err := store.Begin()
 	require.NoError(t, err)
 	historyJobs, err := ddl.GetLastNHistoryDDLJobs(meta.NewMeta(txn), ddl.DefNumHistoryJobs)
 	require.Greater(t, len(historyJobs), 1)