From cb2a425eb5f817c69110fdedf83b945d19a67087 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 3 Sep 2024 14:52:50 +0800 Subject: [PATCH 01/17] ddl, stats: switch to new struct for create/truncate table Signed-off-by: lance6716 --- pkg/ddl/create_table.go | 38 ++++++------ pkg/ddl/table.go | 13 ++-- pkg/ddl/util/schema_change_notifier.go | 80 ++++++++++++++++++++++++- pkg/statistics/handle/ddl/ddl.go | 74 +++++++++++------------ pkg/statistics/handle/util/ddl_event.go | 38 +----------- 5 files changed, 143 insertions(+), 100 deletions(-) diff --git a/pkg/ddl/create_table.go b/pkg/ddl/create_table.go index d25800058d032..15229099ec89f 100644 --- a/pkg/ddl/create_table.go +++ b/pkg/ddl/create_table.go @@ -27,6 +27,7 @@ import ( "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/ddl/logutil" "github.com/pingcap/tidb/pkg/ddl/placement" + "github.com/pingcap/tidb/pkg/ddl/util" "github.com/pingcap/tidb/pkg/domain/infosync" "github.com/pingcap/tidb/pkg/expression" "github.com/pingcap/tidb/pkg/infoschema" @@ -44,6 +45,7 @@ import ( "github.com/pingcap/tidb/pkg/table/tables" "github.com/pingcap/tidb/pkg/types" driver "github.com/pingcap/tidb/pkg/types/parser_driver" + tidb_util "github.com/pingcap/tidb/pkg/util" "github.com/pingcap/tidb/pkg/util/dbterror" "github.com/pingcap/tidb/pkg/util/mock" "github.com/pingcap/tidb/pkg/util/set" @@ -180,11 +182,12 @@ func onCreateTable(jobCtx *jobContext, t *meta.Meta, job *model.Job) (ver int64, // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) - createTableEvent := statsutil.NewCreateTableEvent( - job.SchemaID, - tbInfo, - ) - asyncNotifyEvent(jobCtx, createTableEvent) + if !tidb_util.IsMemOrSysDB(job.SchemaName) { + createTableEvent := &statsutil.DDLEvent{ + SchemaChangeEvent: util.NewCreateTableEvent(tbInfo), + } + asyncNotifyEvent(jobCtx, createTableEvent) + } return ver, errors.Trace(err) } @@ -212,11 +215,12 @@ func createTableWithForeignKeys(jobCtx *jobContext, t *meta.Meta, job *model.Job return ver, errors.Trace(err) } job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) - createTableEvent := statsutil.NewCreateTableEvent( - job.SchemaID, - tbInfo, - ) - asyncNotifyEvent(jobCtx, createTableEvent) + if !tidb_util.IsMemOrSysDB(job.SchemaName) { + createTableEvent := &statsutil.DDLEvent{ + SchemaChangeEvent: util.NewCreateTableEvent(tbInfo), + } + asyncNotifyEvent(jobCtx, createTableEvent) + } return ver, nil default: return ver, errors.Trace(dbterror.ErrInvalidDDLJob.GenWithStackByArgs("table", tbInfo.State)) @@ -269,13 +273,13 @@ func onCreateTables(jobCtx *jobContext, t *meta.Meta, job *model.Job) (int64, er job.State = model.JobStateDone job.SchemaState = model.StatePublic job.BinlogInfo.SetTableInfos(ver, args) - - for i := range args { - createTableEvent := statsutil.NewCreateTableEvent( - job.SchemaID, - args[i], - ) - asyncNotifyEvent(jobCtx, createTableEvent) + if !tidb_util.IsMemOrSysDB(job.SchemaName) { + for i := range args { + createTableEvent := &statsutil.DDLEvent{ + SchemaChangeEvent: util.NewCreateTableEvent(args[i]), + } + asyncNotifyEvent(jobCtx, createTableEvent) + } } return ver, errors.Trace(err) diff --git a/pkg/ddl/table.go b/pkg/ddl/table.go index b32e9e6e83134..208253cc58d04 100644 --- a/pkg/ddl/table.go +++ b/pkg/ddl/table.go @@ -28,6 +28,7 @@ import ( "github.com/pingcap/tidb/pkg/ddl/logutil" "github.com/pingcap/tidb/pkg/ddl/placement" sess "github.com/pingcap/tidb/pkg/ddl/session" + "github.com/pingcap/tidb/pkg/ddl/util" "github.com/pingcap/tidb/pkg/domain/infosync" "github.com/pingcap/tidb/pkg/infoschema" "github.com/pingcap/tidb/pkg/meta" @@ -568,12 +569,12 @@ func (w *worker) onTruncateTable(jobCtx *jobContext, t *meta.Meta, job *model.Jo return ver, errors.Trace(err) } job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo) - truncateTableEvent := statsutil.NewTruncateTableEvent( - job.SchemaID, - tblInfo, - oldTblInfo, - ) - asyncNotifyEvent(jobCtx, truncateTableEvent) + if !tidb_util.IsMemOrSysDB(job.SchemaName) { + truncateTableEvent := &statsutil.DDLEvent{ + SchemaChangeEvent: util.NewTruncateTableEvent(tblInfo, oldTblInfo), + } + asyncNotifyEvent(jobCtx, truncateTableEvent) + } startKey := tablecodec.EncodeTablePrefix(tableID) job.Args = []any{startKey, oldPartitionIDs} return ver, nil diff --git a/pkg/ddl/util/schema_change_notifier.go b/pkg/ddl/util/schema_change_notifier.go index 714ec0fccac59..7e58a80e35141 100644 --- a/pkg/ddl/util/schema_change_notifier.go +++ b/pkg/ddl/util/schema_change_notifier.go @@ -14,16 +14,90 @@ package util -import "github.com/pingcap/tidb/pkg/parser/model" +import ( + "fmt" + "strings" -// SchemaChangeEvent stands for a schema change event. DDL will generate one event or multiple events (only for multi-schema change DDL). -// The caller should check the Type field of SchemaChange and call the corresponding getter function to retrieve the needed information. + "github.com/pingcap/tidb/pkg/parser/model" + "github.com/pingcap/tidb/pkg/util/intest" +) + +// SchemaChangeEvent stands for a schema change event. DDL will generate one +// event or multiple events (only for multi-schema change DDL). The caller should +// check the GetType of SchemaChange and call the corresponding getter function +// to retrieve the needed information. type SchemaChangeEvent struct { // todo: field and method will be added in the next few pr on demand tp model.ActionType + + newTableInfo *model.TableInfo + oldTableInfo *model.TableInfo +} + +// String implements fmt.Stringer interface. +func (s *SchemaChangeEvent) String() string { + if s == nil { + return "nil SchemaChangeEvent" + } + + var sb strings.Builder + _, _ = fmt.Fprintf(&sb, "(Event Type: %s", s.tp) + if s.newTableInfo != nil { + _, _ = fmt.Fprintf(&sb, ", Table ID: %d, Table Name: %s", s.newTableInfo.ID, s.newTableInfo.Name) + } + if s.oldTableInfo != nil { + _, _ = fmt.Fprintf(&sb, ", Old Table ID: %d, Old Table Name: %s", s.oldTableInfo.ID, s.oldTableInfo.Name) + } + sb.WriteString(")") + + return sb.String() } // GetType returns the type of the schema change event. func (s *SchemaChangeEvent) GetType() model.ActionType { + if s == nil { + return model.ActionNone + } return s.tp } + +// NewCreateTableEvent creates a SchemaChangeEvent whose type is +// ActionCreateTable. +func NewCreateTableEvent( + newTableInfo *model.TableInfo, +) *SchemaChangeEvent { + return &SchemaChangeEvent{ + tp: model.ActionCreateTable, + newTableInfo: newTableInfo, + } +} + +// GetCreateTableInfo returns the table info of the SchemaChangeEvent whose type +// is ActionCreateTable. +func (s *SchemaChangeEvent) GetCreateTableInfo() *model.TableInfo { + intest.Assert(s.tp == model.ActionCreateTable) + return s.newTableInfo +} + +// NewTruncateTableEvent creates a SchemaChangeEvent whose type is +// ActionTruncateTable. +func NewTruncateTableEvent( + newTableInfo *model.TableInfo, + droppedTableInfo *model.TableInfo, +) *SchemaChangeEvent { + return &SchemaChangeEvent{ + tp: model.ActionTruncateTable, + newTableInfo: newTableInfo, + oldTableInfo: droppedTableInfo, + } +} + +// GetTruncateTableInfo returns the new and old table info of the +// SchemaChangeEvent whose type is ActionTruncateTable. +func (s *SchemaChangeEvent) GetTruncateTableInfo() ( + newTableInfo *model.TableInfo, + droppedTableInfo *model.TableInfo, +) { + intest.Assert(s.tp == model.ActionTruncateTable) + return s.newTableInfo, s.oldTableInfo +} diff --git a/pkg/statistics/handle/ddl/ddl.go b/pkg/statistics/handle/ddl/ddl.go index 3a2278f0fa8cd..e6b0272b70e11 100644 --- a/pkg/statistics/handle/ddl/ddl.go +++ b/pkg/statistics/handle/ddl/ddl.go @@ -78,39 +78,6 @@ func (h *ddlHandlerImpl) HandleDDLEvent(t *util.DDLEvent) error { logutil.StatsLogger().Info("Handle ddl event", zap.Stringer("event", t)) switch t.GetType() { - case model.ActionCreateTable: - newTableInfo := t.GetCreateTableInfo() - ids, err := h.getTableIDs(newTableInfo) - if err != nil { - return err - } - for _, id := range ids { - if err := h.statsWriter.InsertTableStats2KV(newTableInfo, id); err != nil { - return err - } - } - case model.ActionTruncateTable: - newTableInfo, droppedTableInfo := t.GetTruncateTableInfo() - ids, err := h.getTableIDs(newTableInfo) - if err != nil { - return err - } - for _, id := range ids { - if err := h.statsWriter.InsertTableStats2KV(newTableInfo, id); err != nil { - return err - } - } - - // Remove the old table stats. - droppedIDs, err := h.getTableIDs(droppedTableInfo) - if err != nil { - return err - } - for _, id := range droppedIDs { - if err := h.statsWriter.UpdateStatsMetaVersionForGC(id); err != nil { - return err - } - } case model.ActionDropTable: droppedTableInfo := t.GetDropTableInfo() ids, err := h.getTableIDs(droppedTableInfo) @@ -197,13 +164,46 @@ func (h *ddlHandlerImpl) HandleDDLEvent(t *util.DDLEvent) error { } case model.ActionFlashbackCluster: return h.statsWriter.UpdateStatsVersion() + default: + logutil.StatsLogger().Info("Handle schema change event", zap.Stringer("event", t.SchemaChangeEvent)) } - //revive:disable:empty-block - switch t.SchemaChangeEvent.GetType() { - // todo: we will replace the DDLEvent with SchemaChangeEvent, gradually move above switch-case logical to here + e := t.SchemaChangeEvent + switch e.GetType() { + case model.ActionCreateTable: + newTableInfo := e.GetCreateTableInfo() + ids, err := h.getTableIDs(newTableInfo) + if err != nil { + return err + } + for _, id := range ids { + if err := h.statsWriter.InsertTableStats2KV(newTableInfo, id); err != nil { + return err + } + } + case model.ActionTruncateTable: + newTableInfo, droppedTableInfo := e.GetTruncateTableInfo() + ids, err := h.getTableIDs(newTableInfo) + if err != nil { + return err + } + for _, id := range ids { + if err := h.statsWriter.InsertTableStats2KV(newTableInfo, id); err != nil { + return err + } + } + + // Remove the old table stats. + droppedIDs, err := h.getTableIDs(droppedTableInfo) + if err != nil { + return err + } + for _, id := range droppedIDs { + if err := h.statsWriter.UpdateStatsMetaVersionForGC(id); err != nil { + return err + } + } } - //revive:enable:empty-block return nil } diff --git a/pkg/statistics/handle/util/ddl_event.go b/pkg/statistics/handle/util/ddl_event.go index 9917a451bb663..bf2ce11e73435 100644 --- a/pkg/statistics/handle/util/ddl_event.go +++ b/pkg/statistics/handle/util/ddl_event.go @@ -47,7 +47,7 @@ type DDLEvent struct { tp model.ActionType // todo: replace DDLEvent by SchemaChangeEvent gradually - SchemaChangeEvent ddlutil.SchemaChangeEvent + SchemaChangeEvent *ddlutil.SchemaChangeEvent } // IsMemOrSysDB checks whether the table is in the memory or system database. @@ -61,42 +61,6 @@ func (e *DDLEvent) IsMemOrSysDB(sctx sessionctx.Context) (bool, error) { return util.IsMemOrSysDB(schema.Name.L), nil } -// NewCreateTableEvent creates a new ddl event that creates a table. -func NewCreateTableEvent( - schemaID int64, - newTableInfo *model.TableInfo, -) *DDLEvent { - return &DDLEvent{ - tp: model.ActionCreateTable, - schemaID: schemaID, - tableInfo: newTableInfo, - } -} - -// GetCreateTableInfo gets the table info of the table that is created. -func (e *DDLEvent) GetCreateTableInfo() (newTableInfo *model.TableInfo) { - return e.tableInfo -} - -// NewTruncateTableEvent creates a new ddl event that truncates a table. -func NewTruncateTableEvent( - schemaID int64, - newTableInfo *model.TableInfo, - droppedTableInfo *model.TableInfo, -) *DDLEvent { - return &DDLEvent{ - tp: model.ActionTruncateTable, - schemaID: schemaID, - tableInfo: newTableInfo, - oldTableInfo: droppedTableInfo, - } -} - -// GetTruncateTableInfo gets the table info of the table that is truncated. -func (e *DDLEvent) GetTruncateTableInfo() (newTableInfo *model.TableInfo, droppedTableInfo *model.TableInfo) { - return e.tableInfo, e.oldTableInfo -} - // NewDropTableEvent creates a new ddl event that drops a table. func NewDropTableEvent( schemaID int64, From b9178cde7f9129264683260fd298741bb40ce8e1 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 3 Sep 2024 15:04:35 +0800 Subject: [PATCH 02/17] fix CI Signed-off-by: lance6716 --- pkg/ddl/util/BUILD.bazel | 1 + pkg/ddl/util/schema_change_notifier.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/ddl/util/BUILD.bazel b/pkg/ddl/util/BUILD.bazel index 115d40aa55774..89baacc3f862c 100644 --- a/pkg/ddl/util/BUILD.bazel +++ b/pkg/ddl/util/BUILD.bazel @@ -22,6 +22,7 @@ go_library( "//pkg/sessionctx/variable", "//pkg/table/tables", "//pkg/util/chunk", + "//pkg/util/intest", "//pkg/util/mock", "//pkg/util/sqlexec", "@com_github_pingcap_errors//:errors", diff --git a/pkg/ddl/util/schema_change_notifier.go b/pkg/ddl/util/schema_change_notifier.go index 7e58a80e35141..49eebc091842e 100644 --- a/pkg/ddl/util/schema_change_notifier.go +++ b/pkg/ddl/util/schema_change_notifier.go @@ -28,10 +28,10 @@ import ( // to retrieve the needed information. type SchemaChangeEvent struct { // todo: field and method will be added in the next few pr on demand - tp model.ActionType - newTableInfo *model.TableInfo oldTableInfo *model.TableInfo + + tp model.ActionType } // String implements fmt.Stringer interface. From d5e92d99b58aef6d7a3672fd56b0d716eab75c8d Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 3 Sep 2024 15:16:15 +0800 Subject: [PATCH 03/17] fix lint Signed-off-by: lance6716 --- pkg/statistics/handle/util/ddl_event.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/statistics/handle/util/ddl_event.go b/pkg/statistics/handle/util/ddl_event.go index bf2ce11e73435..3c274ddbf698b 100644 --- a/pkg/statistics/handle/util/ddl_event.go +++ b/pkg/statistics/handle/util/ddl_event.go @@ -38,6 +38,9 @@ type DDLEvent struct { oldTableInfo *model.TableInfo oldPartInfo *model.PartitionInfo columnInfos []*model.ColumnInfo + // todo: replace DDLEvent by SchemaChangeEvent gradually + SchemaChangeEvent *ddlutil.SchemaChangeEvent + // schemaID is the ID of the schema that the table belongs to. // Used to filter out the system or memory tables. schemaID int64 @@ -45,9 +48,6 @@ type DDLEvent struct { // It applies when a table structure is being changed from partitioned to non-partitioned, or vice versa. oldTableID int64 tp model.ActionType - - // todo: replace DDLEvent by SchemaChangeEvent gradually - SchemaChangeEvent *ddlutil.SchemaChangeEvent } // IsMemOrSysDB checks whether the table is in the memory or system database. From 282610c44314e02e0a3960e35036a5d7991ca6d7 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 3 Sep 2024 15:24:02 +0800 Subject: [PATCH 04/17] make fmt Signed-off-by: lance6716 --- pkg/statistics/handle/util/ddl_event.go | 2 +- pkg/util/collate/unicode_0400_ci_generated.go | 2 +- pkg/util/collate/unicode_0900_ai_ci_generated.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/statistics/handle/util/ddl_event.go b/pkg/statistics/handle/util/ddl_event.go index 3c274ddbf698b..e2b891b5eb50f 100644 --- a/pkg/statistics/handle/util/ddl_event.go +++ b/pkg/statistics/handle/util/ddl_event.go @@ -40,7 +40,7 @@ type DDLEvent struct { columnInfos []*model.ColumnInfo // todo: replace DDLEvent by SchemaChangeEvent gradually SchemaChangeEvent *ddlutil.SchemaChangeEvent - + // schemaID is the ID of the schema that the table belongs to. // Used to filter out the system or memory tables. schemaID int64 diff --git a/pkg/util/collate/unicode_0400_ci_generated.go b/pkg/util/collate/unicode_0400_ci_generated.go index c073d4c67aad7..db1ac70e218e9 100644 --- a/pkg/util/collate/unicode_0400_ci_generated.go +++ b/pkg/util/collate/unicode_0400_ci_generated.go @@ -25,7 +25,7 @@ type unicodeCICollator struct { // Clone implements Collator interface. func (uc *unicodeCICollator) Clone() Collator { - return &unicodeCICollator{impl: uc.impl.Clone()} + return &unicodeCICollator{impl: uc.impl.Clone()} } // Compare implements Collator interface. diff --git a/pkg/util/collate/unicode_0900_ai_ci_generated.go b/pkg/util/collate/unicode_0900_ai_ci_generated.go index 5e160c6182231..51177f893ca9c 100644 --- a/pkg/util/collate/unicode_0900_ai_ci_generated.go +++ b/pkg/util/collate/unicode_0900_ai_ci_generated.go @@ -25,7 +25,7 @@ type unicode0900AICICollator struct { // Clone implements Collator interface. func (uc *unicode0900AICICollator) Clone() Collator { - return &unicode0900AICICollator{impl: uc.impl.Clone()} + return &unicode0900AICICollator{impl: uc.impl.Clone()} } // Compare implements Collator interface. From b74435d0a405a3471e544e757c10710694b1bd69 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 3 Sep 2024 15:27:56 +0800 Subject: [PATCH 05/17] revert some change Signed-off-by: lance6716 --- pkg/util/collate/unicode_0400_ci_generated.go | 2 +- pkg/util/collate/unicode_0900_ai_ci_generated.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/util/collate/unicode_0400_ci_generated.go b/pkg/util/collate/unicode_0400_ci_generated.go index db1ac70e218e9..c073d4c67aad7 100644 --- a/pkg/util/collate/unicode_0400_ci_generated.go +++ b/pkg/util/collate/unicode_0400_ci_generated.go @@ -25,7 +25,7 @@ type unicodeCICollator struct { // Clone implements Collator interface. func (uc *unicodeCICollator) Clone() Collator { - return &unicodeCICollator{impl: uc.impl.Clone()} + return &unicodeCICollator{impl: uc.impl.Clone()} } // Compare implements Collator interface. diff --git a/pkg/util/collate/unicode_0900_ai_ci_generated.go b/pkg/util/collate/unicode_0900_ai_ci_generated.go index 51177f893ca9c..5e160c6182231 100644 --- a/pkg/util/collate/unicode_0900_ai_ci_generated.go +++ b/pkg/util/collate/unicode_0900_ai_ci_generated.go @@ -25,7 +25,7 @@ type unicode0900AICICollator struct { // Clone implements Collator interface. func (uc *unicode0900AICICollator) Clone() Collator { - return &unicode0900AICICollator{impl: uc.impl.Clone()} + return &unicode0900AICICollator{impl: uc.impl.Clone()} } // Compare implements Collator interface. From faf1dd9772fa935c3df747c212aa5668c0f0358f Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 3 Sep 2024 15:39:27 +0800 Subject: [PATCH 06/17] fix lint Signed-off-by: lance6716 --- pkg/statistics/handle/util/ddl_event.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/statistics/handle/util/ddl_event.go b/pkg/statistics/handle/util/ddl_event.go index e2b891b5eb50f..be0d4b6cc31c5 100644 --- a/pkg/statistics/handle/util/ddl_event.go +++ b/pkg/statistics/handle/util/ddl_event.go @@ -29,6 +29,8 @@ import ( // DDLEvent contains the information of a ddl event that is used to update stats. type DDLEvent struct { + // todo: replace DDLEvent by SchemaChangeEvent gradually + SchemaChangeEvent *ddlutil.SchemaChangeEvent // For different ddl types, the following fields are used. // They have different meanings for different ddl types. // Please do **not** use these fields directly, use the corresponding @@ -38,8 +40,6 @@ type DDLEvent struct { oldTableInfo *model.TableInfo oldPartInfo *model.PartitionInfo columnInfos []*model.ColumnInfo - // todo: replace DDLEvent by SchemaChangeEvent gradually - SchemaChangeEvent *ddlutil.SchemaChangeEvent // schemaID is the ID of the schema that the table belongs to. // Used to filter out the system or memory tables. From a5eee63b256902905526cf1b8392e64e6531d5c3 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 3 Sep 2024 15:53:32 +0800 Subject: [PATCH 07/17] fix CI Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/ddl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/statistics/handle/ddl/ddl.go b/pkg/statistics/handle/ddl/ddl.go index e6b0272b70e11..7f930574df705 100644 --- a/pkg/statistics/handle/ddl/ddl.go +++ b/pkg/statistics/handle/ddl/ddl.go @@ -58,7 +58,7 @@ func (h *ddlHandlerImpl) HandleDDLEvent(t *util.DDLEvent) error { // ActionFlashbackCluster will not create any new stats info // and it's SchemaID alwayws equals to 0, so skip check it. - if t.GetType() != model.ActionFlashbackCluster { + if t.GetType() != model.ActionFlashbackCluster && t.SchemaChangeEvent == nil { if isSysDB, err := t.IsMemOrSysDB(sctx.(sessionctx.Context)); err != nil { return err } else if isSysDB { From a9d89b257307e224603e7d3cd5226cf3b0ea587b Mon Sep 17 00:00:00 2001 From: lance6716 Date: Wed, 4 Sep 2024 10:11:41 +0800 Subject: [PATCH 08/17] address comment Signed-off-by: lance6716 --- pkg/ddl/create_table.go | 8 ++++---- pkg/ddl/table.go | 6 +++--- pkg/session/bootstraptest/bootstrap_upgrade_test.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/ddl/create_table.go b/pkg/ddl/create_table.go index 15229099ec89f..f31205eed3847 100644 --- a/pkg/ddl/create_table.go +++ b/pkg/ddl/create_table.go @@ -45,7 +45,7 @@ import ( "github.com/pingcap/tidb/pkg/table/tables" "github.com/pingcap/tidb/pkg/types" driver "github.com/pingcap/tidb/pkg/types/parser_driver" - tidb_util "github.com/pingcap/tidb/pkg/util" + tidbutil "github.com/pingcap/tidb/pkg/util" "github.com/pingcap/tidb/pkg/util/dbterror" "github.com/pingcap/tidb/pkg/util/mock" "github.com/pingcap/tidb/pkg/util/set" @@ -182,7 +182,7 @@ func onCreateTable(jobCtx *jobContext, t *meta.Meta, job *model.Job) (ver int64, // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) - if !tidb_util.IsMemOrSysDB(job.SchemaName) { + if !tidbutil.IsMemOrSysDB(job.SchemaName) { createTableEvent := &statsutil.DDLEvent{ SchemaChangeEvent: util.NewCreateTableEvent(tbInfo), } @@ -215,7 +215,7 @@ func createTableWithForeignKeys(jobCtx *jobContext, t *meta.Meta, job *model.Job return ver, errors.Trace(err) } job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) - if !tidb_util.IsMemOrSysDB(job.SchemaName) { + if !tidbutil.IsMemOrSysDB(job.SchemaName) { createTableEvent := &statsutil.DDLEvent{ SchemaChangeEvent: util.NewCreateTableEvent(tbInfo), } @@ -273,7 +273,7 @@ func onCreateTables(jobCtx *jobContext, t *meta.Meta, job *model.Job) (int64, er job.State = model.JobStateDone job.SchemaState = model.StatePublic job.BinlogInfo.SetTableInfos(ver, args) - if !tidb_util.IsMemOrSysDB(job.SchemaName) { + if !tidbutil.IsMemOrSysDB(job.SchemaName) { for i := range args { createTableEvent := &statsutil.DDLEvent{ SchemaChangeEvent: util.NewCreateTableEvent(args[i]), diff --git a/pkg/ddl/table.go b/pkg/ddl/table.go index 208253cc58d04..a6b698b9c04a5 100644 --- a/pkg/ddl/table.go +++ b/pkg/ddl/table.go @@ -42,7 +42,7 @@ import ( "github.com/pingcap/tidb/pkg/table" "github.com/pingcap/tidb/pkg/table/tables" "github.com/pingcap/tidb/pkg/tablecodec" - tidb_util "github.com/pingcap/tidb/pkg/util" + tidbutil "github.com/pingcap/tidb/pkg/util" "github.com/pingcap/tidb/pkg/util/dbterror" "github.com/pingcap/tidb/pkg/util/gcutil" "go.uber.org/zap" @@ -569,7 +569,7 @@ func (w *worker) onTruncateTable(jobCtx *jobContext, t *meta.Meta, job *model.Jo return ver, errors.Trace(err) } job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo) - if !tidb_util.IsMemOrSysDB(job.SchemaName) { + if !tidbutil.IsMemOrSysDB(job.SchemaName) { truncateTableEvent := &statsutil.DDLEvent{ SchemaChangeEvent: util.NewTruncateTableEvent(tblInfo, oldTblInfo), } @@ -1074,7 +1074,7 @@ func (w *worker) onSetTableFlashReplica(jobCtx *jobContext, t *meta.Meta, job *m } // Ban setting replica count for tables in system database. - if tidb_util.IsMemOrSysDB(job.SchemaName) { + if tidbutil.IsMemOrSysDB(job.SchemaName) { return ver, errors.Trace(dbterror.ErrUnsupportedTiFlashOperationForSysOrMemTable) } diff --git a/pkg/session/bootstraptest/bootstrap_upgrade_test.go b/pkg/session/bootstraptest/bootstrap_upgrade_test.go index 28cf75f22a709..1e5ac45f43668 100644 --- a/pkg/session/bootstraptest/bootstrap_upgrade_test.go +++ b/pkg/session/bootstraptest/bootstrap_upgrade_test.go @@ -37,7 +37,7 @@ import ( "github.com/pingcap/tidb/pkg/sessionctx" "github.com/pingcap/tidb/pkg/testkit" "github.com/pingcap/tidb/pkg/testkit/testfailpoint" - tidb_util "github.com/pingcap/tidb/pkg/util" + tidbutil "github.com/pingcap/tidb/pkg/util" "github.com/pingcap/tidb/pkg/util/chunk" "github.com/pingcap/tidb/pkg/util/sqlexec" "github.com/stretchr/testify/require" @@ -718,7 +718,7 @@ func TestUpgradeWithPauseDDL(t *testing.T) { require.NoError(t, err) cmt := fmt.Sprintf("job: %s", runJob.String()) isPause := runJob.IsPausedBySystem() || runJob.IsPausing() - if tidb_util.IsSysDB(runJob.SchemaName) { + if tidbutil.IsSysDB(runJob.SchemaName) { require.False(t, isPause, cmt) } else { require.True(t, isPause, cmt) From 249d1227d0b5b5d7e020aa6437473486fe9ee865 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 10:05:14 +0800 Subject: [PATCH 09/17] address comment Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/ddl.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/statistics/handle/ddl/ddl.go b/pkg/statistics/handle/ddl/ddl.go index 7f930574df705..871d7babc0663 100644 --- a/pkg/statistics/handle/ddl/ddl.go +++ b/pkg/statistics/handle/ddl/ddl.go @@ -75,7 +75,11 @@ func (h *ddlHandlerImpl) HandleDDLEvent(t *util.DDLEvent) error { return nil } } - logutil.StatsLogger().Info("Handle ddl event", zap.Stringer("event", t)) + if t.SchemaChangeEvent == nil { + // when SchemaChangeEvent is set, it will be printed in the default branch of + // below switch. + logutil.StatsLogger().Info("Handle ddl event", zap.Stringer("event", t)) + } switch t.GetType() { case model.ActionDropTable: From a0a25a42f4dac2d9f50a9d3f92ae952fbd38be2a Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 10:50:00 +0800 Subject: [PATCH 10/17] address comment Signed-off-by: lance6716 --- pkg/ddl/add_column.go | 2 +- pkg/ddl/cluster.go | 4 ++-- pkg/ddl/create_table.go | 27 ++++++++++----------------- pkg/ddl/ddl.go | 9 ++++++++- pkg/ddl/modify_column.go | 2 +- pkg/ddl/partition.go | 12 ++++++------ pkg/ddl/table.go | 10 ++++------ 7 files changed, 32 insertions(+), 34 deletions(-) diff --git a/pkg/ddl/add_column.go b/pkg/ddl/add_column.go index 396c2cc04c2f9..3b6db8f386190 100644 --- a/pkg/ddl/add_column.go +++ b/pkg/ddl/add_column.go @@ -137,7 +137,7 @@ func onAddColumn(jobCtx *jobContext, t *meta.Meta, job *model.Job) (ver int64, e tblInfo, []*model.ColumnInfo{columnInfo}, ) - asyncNotifyEvent(jobCtx, addColumnEvent) + asyncNotifyEvent(jobCtx, addColumnEvent, job) default: err = dbterror.ErrInvalidDDLState.GenWithStackByArgs("column", columnInfo.State) } diff --git a/pkg/ddl/cluster.go b/pkg/ddl/cluster.go index 93e8e4849abc9..b52deef85a6d2 100644 --- a/pkg/ddl/cluster.go +++ b/pkg/ddl/cluster.go @@ -821,7 +821,7 @@ func (w *worker) onFlashbackCluster(jobCtx *jobContext, t *meta.Meta, job *model case model.StateWriteReorganization: // TODO: Support flashback in unistore. if inFlashbackTest { - asyncNotifyEvent(jobCtx, statsutil.NewFlashbackClusterEvent()) + asyncNotifyEvent(jobCtx, statsutil.NewFlashbackClusterEvent(), job) job.State = model.JobStateDone job.SchemaState = model.StatePublic return ver, nil @@ -844,7 +844,7 @@ func (w *worker) onFlashbackCluster(jobCtx *jobContext, t *meta.Meta, job *model } } - asyncNotifyEvent(jobCtx, statsutil.NewFlashbackClusterEvent()) + asyncNotifyEvent(jobCtx, statsutil.NewFlashbackClusterEvent(), job) job.State = model.JobStateDone job.SchemaState = model.StatePublic return updateSchemaVersion(jobCtx, t, job) diff --git a/pkg/ddl/create_table.go b/pkg/ddl/create_table.go index 1db976bf5f066..af4dafa0f7198 100644 --- a/pkg/ddl/create_table.go +++ b/pkg/ddl/create_table.go @@ -46,7 +46,6 @@ import ( "github.com/pingcap/tidb/pkg/table/tables" "github.com/pingcap/tidb/pkg/types" driver "github.com/pingcap/tidb/pkg/types/parser_driver" - tidbutil "github.com/pingcap/tidb/pkg/util" "github.com/pingcap/tidb/pkg/util/dbterror" "github.com/pingcap/tidb/pkg/util/mock" "github.com/pingcap/tidb/pkg/util/set" @@ -183,12 +182,10 @@ func onCreateTable(jobCtx *jobContext, t *meta.Meta, job *model.Job) (ver int64, // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) - if !tidbutil.IsMemOrSysDB(job.SchemaName) { - createTableEvent := &statsutil.DDLEvent{ - SchemaChangeEvent: util.NewCreateTableEvent(tbInfo), - } - asyncNotifyEvent(jobCtx, createTableEvent) + createTableEvent := &statsutil.DDLEvent{ + SchemaChangeEvent: util.NewCreateTableEvent(tbInfo), } + asyncNotifyEvent(jobCtx, createTableEvent, job) return ver, errors.Trace(err) } @@ -216,12 +213,10 @@ func createTableWithForeignKeys(jobCtx *jobContext, t *meta.Meta, job *model.Job return ver, errors.Trace(err) } job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) - if !tidbutil.IsMemOrSysDB(job.SchemaName) { - createTableEvent := &statsutil.DDLEvent{ - SchemaChangeEvent: util.NewCreateTableEvent(tbInfo), - } - asyncNotifyEvent(jobCtx, createTableEvent) + createTableEvent := &statsutil.DDLEvent{ + SchemaChangeEvent: util.NewCreateTableEvent(tbInfo), } + asyncNotifyEvent(jobCtx, createTableEvent, job) return ver, nil default: return ver, errors.Trace(dbterror.ErrInvalidDDLJob.GenWithStackByArgs("table", tbInfo.State)) @@ -274,13 +269,11 @@ func onCreateTables(jobCtx *jobContext, t *meta.Meta, job *model.Job) (int64, er job.State = model.JobStateDone job.SchemaState = model.StatePublic job.BinlogInfo.SetTableInfos(ver, args) - if !tidbutil.IsMemOrSysDB(job.SchemaName) { - for i := range args { - createTableEvent := &statsutil.DDLEvent{ - SchemaChangeEvent: util.NewCreateTableEvent(args[i]), - } - asyncNotifyEvent(jobCtx, createTableEvent) + for i := range args { + createTableEvent := &statsutil.DDLEvent{ + SchemaChangeEvent: util.NewCreateTableEvent(args[i]), } + asyncNotifyEvent(jobCtx, createTableEvent, job) } return ver, errors.Trace(err) diff --git a/pkg/ddl/ddl.go b/pkg/ddl/ddl.go index a671504d717bf..9fb1bcdeec913 100644 --- a/pkg/ddl/ddl.go +++ b/pkg/ddl/ddl.go @@ -556,7 +556,14 @@ func (d *ddl) RegisterStatsHandle(h *handle.Handle) { // asyncNotifyEvent will notify the ddl event to outside world, say statistic handle. When the channel is full, we may // give up notify and log it. -func asyncNotifyEvent(jobCtx *jobContext, e *statsutil.DDLEvent) { +func asyncNotifyEvent(jobCtx *jobContext, e *statsutil.DDLEvent, job *model.Job) { + // skip notify for system databases, system databases are expected to change at + // bootstrap and other nodes can also handle the changing in its bootstrap rather + // than be notified. + if tidbutil.IsMemOrSysDB(job.SchemaName) { + return + } + ch := jobCtx.oldDDLCtx.ddlEventCh if ch != nil { for i := 0; i < 10; i++ { diff --git a/pkg/ddl/modify_column.go b/pkg/ddl/modify_column.go index 2665fdd9c7846..167c1b8e95a4e 100644 --- a/pkg/ddl/modify_column.go +++ b/pkg/ddl/modify_column.go @@ -535,7 +535,7 @@ func (w *worker) doModifyColumnTypeWithData( tblInfo, []*model.ColumnInfo{changingCol}, ) - asyncNotifyEvent(jobCtx, modifyColumnEvent) + asyncNotifyEvent(jobCtx, modifyColumnEvent, job) default: err = dbterror.ErrInvalidDDLState.GenWithStackByArgs("column", changingCol.State) } diff --git a/pkg/ddl/partition.go b/pkg/ddl/partition.go index 32b60954682d7..32bae65319c80 100644 --- a/pkg/ddl/partition.go +++ b/pkg/ddl/partition.go @@ -234,7 +234,7 @@ func (w *worker) onAddTablePartition(jobCtx *jobContext, t *meta.Meta, job *mode tblInfo, partInfo, ) - asyncNotifyEvent(jobCtx, addPartitionEvent) + asyncNotifyEvent(jobCtx, addPartitionEvent, job) default: err = dbterror.ErrInvalidDDLState.GenWithStackByArgs("partition", job.SchemaState) } @@ -2338,7 +2338,7 @@ func (w *worker) onDropTablePartition(jobCtx *jobContext, t *meta.Meta, job *mod tblInfo, &model.PartitionInfo{Definitions: droppedDefs}, ) - asyncNotifyEvent(jobCtx, dropPartitionEvent) + asyncNotifyEvent(jobCtx, dropPartitionEvent, job) // A background job will be created to delete old partition data. job.Args = []any{physicalTableIDs} default: @@ -2431,7 +2431,7 @@ func (w *worker) onTruncateTablePartition(jobCtx *jobContext, t *meta.Meta, job &model.PartitionInfo{Definitions: newPartitions}, &model.PartitionInfo{Definitions: oldPartitions}, ) - asyncNotifyEvent(jobCtx, truncatePartitionEvent) + asyncNotifyEvent(jobCtx, truncatePartitionEvent, job) // A background job will be created to delete old partition data. job.Args = []any{oldIDs} @@ -2570,7 +2570,7 @@ func (w *worker) onTruncateTablePartition(jobCtx *jobContext, t *meta.Meta, job &model.PartitionInfo{Definitions: newPartitions}, &model.PartitionInfo{Definitions: oldPartitions}, ) - asyncNotifyEvent(jobCtx, truncatePartitionEvent) + asyncNotifyEvent(jobCtx, truncatePartitionEvent, job) // A background job will be created to delete old partition data. job.Args = []any{oldIDs} default: @@ -2943,7 +2943,7 @@ func (w *worker) onExchangeTablePartition(jobCtx *jobContext, t *meta.Meta, job &model.PartitionInfo{Definitions: []model.PartitionDefinition{originalPartitionDef}}, originalNt, ) - asyncNotifyEvent(jobCtx, exchangePartitionEvent) + asyncNotifyEvent(jobCtx, exchangePartitionEvent, job) return ver, nil } @@ -3481,7 +3481,7 @@ func (w *worker) onReorganizePartition(jobCtx *jobContext, t *meta.Meta, job *mo if err != nil { return ver, errors.Trace(err) } - asyncNotifyEvent(jobCtx, event) + asyncNotifyEvent(jobCtx, event, job) // A background job will be created to delete old partition data. job.Args = []any{physicalTableIDs} diff --git a/pkg/ddl/table.go b/pkg/ddl/table.go index d01fee35d5693..954877bd8361c 100644 --- a/pkg/ddl/table.go +++ b/pkg/ddl/table.go @@ -128,7 +128,7 @@ func onDropTableOrView(jobCtx *jobContext, t *meta.Meta, job *model.Job) (ver in job.SchemaID, tblInfo, ) - asyncNotifyEvent(jobCtx, dropTableEvent) + asyncNotifyEvent(jobCtx, dropTableEvent, job) } default: return ver, errors.Trace(dbterror.ErrInvalidDDLState.GenWithStackByArgs("table", tblInfo.State)) @@ -570,12 +570,10 @@ func (w *worker) onTruncateTable(jobCtx *jobContext, t *meta.Meta, job *model.Jo return ver, errors.Trace(err) } job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo) - if !tidbutil.IsMemOrSysDB(job.SchemaName) { - truncateTableEvent := &statsutil.DDLEvent{ - SchemaChangeEvent: util.NewTruncateTableEvent(tblInfo, oldTblInfo), - } - asyncNotifyEvent(jobCtx, truncateTableEvent) + truncateTableEvent := &statsutil.DDLEvent{ + SchemaChangeEvent: util.NewTruncateTableEvent(tblInfo, oldTblInfo), } + asyncNotifyEvent(jobCtx, truncateTableEvent, job) startKey := tablecodec.EncodeTablePrefix(tableID) job.Args = []any{startKey, oldPartitionIDs} return ver, nil From 2ccb822166dec73f105c2c6af5ec726a4cc3182e Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 11:55:09 +0800 Subject: [PATCH 11/17] fix UT stuck Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/ddl_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/statistics/handle/ddl/ddl_test.go b/pkg/statistics/handle/ddl/ddl_test.go index df036e1d9b499..d5ad3258b9380 100644 --- a/pkg/statistics/handle/ddl/ddl_test.go +++ b/pkg/statistics/handle/ddl/ddl_test.go @@ -1511,6 +1511,9 @@ func findEvent(eventCh <-chan *util.DDLEvent, eventType model.ActionType) *util. // Find the target event. for { event := <-eventCh + if event.SchemaChangeEvent.GetType() == eventType { + return event + } if event.GetType() == eventType { return event } From 1dfd0dcc8d44ad5080cd19017e94cd6db5c6ddf4 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 13:06:25 +0800 Subject: [PATCH 12/17] remove a UT Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/ddl_test.go | 28 --------------------------- 1 file changed, 28 deletions(-) diff --git a/pkg/statistics/handle/ddl/ddl_test.go b/pkg/statistics/handle/ddl/ddl_test.go index d5ad3258b9380..64aaac6d9f179 100644 --- a/pkg/statistics/handle/ddl/ddl_test.go +++ b/pkg/statistics/handle/ddl/ddl_test.go @@ -328,34 +328,6 @@ func TestRemovePartitioningOfASystemTable(t *testing.T) { require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") } -func TestTruncateAPartitionOfASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - h := do.StatsHandle() - testKit.MustExec("use test") - // Test truncate a partition of a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int) partition by range (c1) (partition p0 values less than (6), partition p1 values less than (11))") - // Truncate partition p1. - testKit.MustExec("alter table mysql.test truncate partition p1") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - // Find the truncate partition event. - truncatePartitionEvent := findEvent(h.DDLEventCh(), model.ActionTruncateTablePartition) - err = h.HandleDDLEvent(truncatePartitionEvent) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - statsTbl := h.GetTableStats(tableInfo) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") - // Check the partitions' stats. - pi := tableInfo.GetPartitionInfo() - for _, def := range pi.Definitions { - statsTbl := h.GetPartitionStats(tableInfo, def.ID) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") - } -} - func TestTruncateTable(t *testing.T) { store, do := testkit.CreateMockStoreAndDomain(t) testKit := testkit.NewTestKit(t, store) From d256a280c07042c50c6b31a03dbaadd7198ed189 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 13:30:48 +0800 Subject: [PATCH 13/17] fix bazel Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/statistics/handle/ddl/BUILD.bazel b/pkg/statistics/handle/ddl/BUILD.bazel index b833cf1204fc0..184fa28bba8d8 100644 --- a/pkg/statistics/handle/ddl/BUILD.bazel +++ b/pkg/statistics/handle/ddl/BUILD.bazel @@ -31,7 +31,7 @@ go_test( timeout = "short", srcs = ["ddl_test.go"], flaky = True, - shard_count = 27, + shard_count = 26, deps = [ "//pkg/meta/model", "//pkg/parser/model", From 8481c153aa7e0da26c748f142565011d141a928a Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 15:25:07 +0800 Subject: [PATCH 14/17] remove more UT Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/ddl_test.go | 39 --------------------------- 1 file changed, 39 deletions(-) diff --git a/pkg/statistics/handle/ddl/ddl_test.go b/pkg/statistics/handle/ddl/ddl_test.go index 64aaac6d9f179..e37c6a8412634 100644 --- a/pkg/statistics/handle/ddl/ddl_test.go +++ b/pkg/statistics/handle/ddl/ddl_test.go @@ -114,45 +114,6 @@ func TestDDLTable(t *testing.T) { require.False(t, statsTbl.Pseudo) } -func TestCreateASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - testKit.MustExec("use test") - // Test create a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int)") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - h := do.StatsHandle() - err = h.HandleDDLEvent(<-h.DDLEventCh()) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - statsTbl := h.GetTableStats(tableInfo) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") -} - -func TestTruncateASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - testKit.MustExec("use test") - // Test truncate a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int)") - testKit.MustExec("truncate table mysql.test") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - h := do.StatsHandle() - // Find the truncate table partition event. - truncateTableEvent := findEvent(h.DDLEventCh(), model.ActionTruncateTable) - err = h.HandleDDLEvent(truncateTableEvent) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - statsTbl := h.GetTableStats(tableInfo) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") -} - func TestDropASystemTable(t *testing.T) { store, do := testkit.CreateMockStoreAndDomain(t) testKit := testkit.NewTestKit(t, store) From c1f4fa13606f025674863c68b4f01d2c01dc83f6 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 15:25:22 +0800 Subject: [PATCH 15/17] fix bazel Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/statistics/handle/ddl/BUILD.bazel b/pkg/statistics/handle/ddl/BUILD.bazel index 184fa28bba8d8..8033e1b2da5d9 100644 --- a/pkg/statistics/handle/ddl/BUILD.bazel +++ b/pkg/statistics/handle/ddl/BUILD.bazel @@ -31,7 +31,7 @@ go_test( timeout = "short", srcs = ["ddl_test.go"], flaky = True, - shard_count = 26, + shard_count = 24, deps = [ "//pkg/meta/model", "//pkg/parser/model", From 8adc9e6a124e7ce058e920d54ef32d311773cc0e Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 15:30:33 +0800 Subject: [PATCH 16/17] change test content Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/BUILD.bazel | 2 +- pkg/statistics/handle/ddl/ddl_test.go | 33 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/pkg/statistics/handle/ddl/BUILD.bazel b/pkg/statistics/handle/ddl/BUILD.bazel index 8033e1b2da5d9..b833cf1204fc0 100644 --- a/pkg/statistics/handle/ddl/BUILD.bazel +++ b/pkg/statistics/handle/ddl/BUILD.bazel @@ -31,7 +31,7 @@ go_test( timeout = "short", srcs = ["ddl_test.go"], flaky = True, - shard_count = 24, + shard_count = 27, deps = [ "//pkg/meta/model", "//pkg/parser/model", diff --git a/pkg/statistics/handle/ddl/ddl_test.go b/pkg/statistics/handle/ddl/ddl_test.go index e37c6a8412634..57ad0ff26d00f 100644 --- a/pkg/statistics/handle/ddl/ddl_test.go +++ b/pkg/statistics/handle/ddl/ddl_test.go @@ -114,6 +114,27 @@ func TestDDLTable(t *testing.T) { require.False(t, statsTbl.Pseudo) } +func TestCreateASystemTable(t *testing.T) { + store, do := testkit.CreateMockStoreAndDomain(t) + testKit := testkit.NewTestKit(t, store) + testKit.MustExec("use test") + // Test create a system table. + testKit.MustExec("create table mysql.test (c1 int, c2 int)") + h := do.StatsHandle() + require.Len(t, h.DDLEventCh(), 0) +} + +func TestTruncateASystemTable(t *testing.T) { + store, do := testkit.CreateMockStoreAndDomain(t) + testKit := testkit.NewTestKit(t, store) + testKit.MustExec("use test") + // Test truncate a system table. + testKit.MustExec("create table mysql.test (c1 int, c2 int)") + testKit.MustExec("truncate table mysql.test") + h := do.StatsHandle() + require.Len(t, h.DDLEventCh(), 0) +} + func TestDropASystemTable(t *testing.T) { store, do := testkit.CreateMockStoreAndDomain(t) testKit := testkit.NewTestKit(t, store) @@ -289,6 +310,18 @@ func TestRemovePartitioningOfASystemTable(t *testing.T) { require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") } +func TestTruncateAPartitionOfASystemTable(t *testing.T) { + store, do := testkit.CreateMockStoreAndDomain(t) + testKit := testkit.NewTestKit(t, store) + h := do.StatsHandle() + testKit.MustExec("use test") + // Test truncate a partition of a system table. + testKit.MustExec("create table mysql.test (c1 int, c2 int) partition by range (c1) (partition p0 values less than (6), partition p1 values less than (11))") + // Truncate partition p1. + testKit.MustExec("alter table mysql.test truncate partition p1") + require.Len(t, h.DDLEventCh(), 0) +} + func TestTruncateTable(t *testing.T) { store, do := testkit.CreateMockStoreAndDomain(t) testKit := testkit.NewTestKit(t, store) From 0ebda7e23c2f0ae2cd094900fdad5b8ba7595a0a Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 5 Sep 2024 19:17:01 +0800 Subject: [PATCH 17/17] refactor tests Signed-off-by: lance6716 --- pkg/statistics/handle/ddl/BUILD.bazel | 2 +- pkg/statistics/handle/ddl/ddl_test.go | 207 +++----------------------- 2 files changed, 19 insertions(+), 190 deletions(-) diff --git a/pkg/statistics/handle/ddl/BUILD.bazel b/pkg/statistics/handle/ddl/BUILD.bazel index b833cf1204fc0..4976d6676941e 100644 --- a/pkg/statistics/handle/ddl/BUILD.bazel +++ b/pkg/statistics/handle/ddl/BUILD.bazel @@ -31,7 +31,7 @@ go_test( timeout = "short", srcs = ["ddl_test.go"], flaky = True, - shard_count = 27, + shard_count = 18, deps = [ "//pkg/meta/model", "//pkg/parser/model", diff --git a/pkg/statistics/handle/ddl/ddl_test.go b/pkg/statistics/handle/ddl/ddl_test.go index 57ad0ff26d00f..c85d0cfb97095 100644 --- a/pkg/statistics/handle/ddl/ddl_test.go +++ b/pkg/statistics/handle/ddl/ddl_test.go @@ -114,7 +114,7 @@ func TestDDLTable(t *testing.T) { require.False(t, statsTbl.Pseudo) } -func TestCreateASystemTable(t *testing.T) { +func TestSystemTableDDLHasNoEvent(t *testing.T) { store, do := testkit.CreateMockStoreAndDomain(t) testKit := testkit.NewTestKit(t, store) testKit.MustExec("use test") @@ -122,203 +122,32 @@ func TestCreateASystemTable(t *testing.T) { testKit.MustExec("create table mysql.test (c1 int, c2 int)") h := do.StatsHandle() require.Len(t, h.DDLEventCh(), 0) -} - -func TestTruncateASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - testKit.MustExec("use test") - // Test truncate a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int)") testKit.MustExec("truncate table mysql.test") - h := do.StatsHandle() require.Len(t, h.DDLEventCh(), 0) -} - -func TestDropASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - testKit.MustExec("use test") - // Test drop a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int)") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - tableID := tableInfo.ID - testKit.MustExec("drop table mysql.test") - h := do.StatsHandle() - // Find the drop table partition event. - dropTableEvent := findEvent(h.DDLEventCh(), model.ActionDropTable) - err = h.HandleDDLEvent(dropTableEvent) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - // No stats for the table. - testKit.MustQuery("select count(*) from mysql.stats_meta where table_id = ?", tableID).Check(testkit.Rows("0")) -} - -func TestAddColumnToASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - testKit.MustExec("use test") - // Test add column to a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int)") testKit.MustExec("alter table mysql.test add column c3 int") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - h := do.StatsHandle() - // Find the add column event. - addColumnEvent := findEvent(h.DDLEventCh(), model.ActionAddColumn) - err = h.HandleDDLEvent(addColumnEvent) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - statsTbl := h.GetTableStats(tableInfo) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") -} - -func TestModifyColumnOfASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - testKit.MustExec("use test") - // Test modify column of a system table. - // NOTE: Types have to be different, otherwise it won't trigger the modify column event. - testKit.MustExec("create table mysql.test (c1 varchar(255), c2 int)") - testKit.MustExec("insert into mysql.test values ('1',2)") + require.Len(t, h.DDLEventCh(), 0) testKit.MustExec("alter table mysql.test modify column c1 int") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - h := do.StatsHandle() - // Find the modify column event. - modifyColumnEvent := findEvent(h.DDLEventCh(), model.ActionModifyColumn) - err = h.HandleDDLEvent(modifyColumnEvent) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - statsTbl := h.GetTableStats(tableInfo) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") -} - -func TestAddNewPartitionToASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - testKit.MustExec("use test") - // Test add new partition to a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int) partition by range (c1) (partition p0 values less than (6))") - // Add partition p1. - testKit.MustExec("alter table mysql.test add partition (partition p1 values less than (11))") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - h := do.StatsHandle() - // Find the add partition event. - addPartitionEvent := findEvent(h.DDLEventCh(), model.ActionAddTablePartition) - err = h.HandleDDLEvent(addPartitionEvent) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - statsTbl := h.GetTableStats(tableInfo) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") - // Check the partitions' stats. - pi := tableInfo.GetPartitionInfo() - for _, def := range pi.Definitions { - statsTbl := h.GetPartitionStats(tableInfo, def.ID) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") - } -} + require.Len(t, h.DDLEventCh(), 0) + testKit.MustExec("drop table mysql.test") + require.Len(t, h.DDLEventCh(), 0) -func TestDropPartitionOfASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - h := do.StatsHandle() - testKit.MustExec("use test") - // Test drop partition of a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int) partition by range (c1) (partition p0 values less than (6), partition p1 values less than (11))") - // Drop partition p1. - testKit.MustExec("alter table mysql.test drop partition p1") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - // Find the drop partition event. - dropPartitionEvent := findEvent(h.DDLEventCh(), model.ActionDropTablePartition) - err = h.HandleDDLEvent(dropPartitionEvent) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - statsTbl := h.GetTableStats(tableInfo) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") - // Check the partitions' stats. - pi := tableInfo.GetPartitionInfo() - for _, def := range pi.Definitions { - statsTbl := h.GetPartitionStats(tableInfo, def.ID) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") - } -} + testKit.MustExec("create table mysql.test2 (c1 int, c2 int) partition by range (c1) (partition p0 values less than (6))") + require.Len(t, h.DDLEventCh(), 0) + testKit.MustExec("alter table mysql.test2 add partition (partition p1 values less than (11))") + require.Len(t, h.DDLEventCh(), 0) + testKit.MustExec("alter table mysql.test2 truncate partition p1") + require.Len(t, h.DDLEventCh(), 0) + testKit.MustExec("alter table mysql.test2 drop partition p1") + require.Len(t, h.DDLEventCh(), 0) + testKit.MustExec("alter table mysql.test2 remove partitioning") + require.Len(t, h.DDLEventCh(), 0) -func TestExchangePartitionWithASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - h := do.StatsHandle() - testKit.MustExec("use test") - // Test exchange partition with a system table. testKit.MustExec("create table t (c1 int, c2 int, index idx(c1, c2)) partition by range (c1) (partition p0 values less than (6))") - testKit.MustExec("create table mysql.test (c1 int, c2 int, index idx(c1, c2))") - // Insert some data to table t. - testKit.MustExec("insert into t values (1,2),(2,2)") - // Analyze table t. - testKit.MustExec("analyze table t") - // Insert some data to table mysql.test. - testKit.MustExec("insert into mysql.test values (1,2),(2,2)") + <-h.DDLEventCh() + testKit.MustExec("create table mysql.test3 (c1 int, c2 int, index idx(c1, c2))") // Exchange partition. - testKit.MustExec("alter table t exchange partition p0 with table mysql.test") - // Find the exchange partition event. - exchangePartitionEvent := findEvent(h.DDLEventCh(), model.ActionExchangeTablePartition) - err := h.HandleDDLEvent(exchangePartitionEvent) - require.NoError(t, err) - is := do.InfoSchema() - require.Nil(t, h.Update(context.Background(), is)) - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - require.NoError(t, err) - statsTbl := h.GetTableStats(tableInfo) // NOTE: This is a rare case and the effort required to address it outweighs the benefits, hence it is not prioritized for a fix. - require.False(t, statsTbl.Pseudo, "even we skip the DDL event, but the table ID is still changed, so we can see the stats") -} - -func TestRemovePartitioningOfASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - h := do.StatsHandle() - testKit.MustExec("use test") - // Test remove partitioning of a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int) partition by range (c1) (partition p0 values less than (6))") - // Remove partitioning. - testKit.MustExec("alter table mysql.test remove partitioning") - is := do.InfoSchema() - tbl, err := is.TableByName(context.Background(), pmodel.NewCIStr("mysql"), pmodel.NewCIStr("test")) - require.NoError(t, err) - tableInfo := tbl.Meta() - // Find the remove partitioning event. - removePartitioningEvent := findEvent(h.DDLEventCh(), model.ActionRemovePartitioning) - err = h.HandleDDLEvent(removePartitioningEvent) - require.NoError(t, err) - require.Nil(t, h.Update(context.Background(), is)) - statsTbl := h.GetTableStats(tableInfo) - require.True(t, statsTbl.Pseudo, "we should not collect stats for system tables") -} - -func TestTruncateAPartitionOfASystemTable(t *testing.T) { - store, do := testkit.CreateMockStoreAndDomain(t) - testKit := testkit.NewTestKit(t, store) - h := do.StatsHandle() - testKit.MustExec("use test") - // Test truncate a partition of a system table. - testKit.MustExec("create table mysql.test (c1 int, c2 int) partition by range (c1) (partition p0 values less than (6), partition p1 values less than (11))") - // Truncate partition p1. - testKit.MustExec("alter table mysql.test truncate partition p1") + testKit.MustExec("alter table t exchange partition p0 with table mysql.test3") require.Len(t, h.DDLEventCh(), 0) }