From 807ca052303e6ef075477cf8b46c54827c879d6f Mon Sep 17 00:00:00 2001 From: guo-shaoge Date: Mon, 27 Dec 2021 12:19:49 +0800 Subject: [PATCH] sessionctx: enable IndexMerge by default (#30650) close pingcap/tidb#29597 --- cmd/explaintest/r/explain_indexmerge.result | 22 +-- planner/core/integration_test.go | 4 + planner/core/testdata/plan_suite_out.json | 12 +- session/bootstrap.go | 29 +++- session/bootstrap_test.go | 141 ++++++++++++++++++++ sessionctx/variable/session.go | 2 +- sessionctx/variable/sysvar.go | 2 +- sessionctx/variable/sysvar_test.go | 9 ++ sessionctx/variable/tidb_vars.go | 1 + 9 files changed, 204 insertions(+), 18 deletions(-) diff --git a/cmd/explaintest/r/explain_indexmerge.result b/cmd/explaintest/r/explain_indexmerge.result index 83bc89a593e7c..40e0cb4a1159d 100644 --- a/cmd/explaintest/r/explain_indexmerge.result +++ b/cmd/explaintest/r/explain_indexmerge.result @@ -6,19 +6,23 @@ create index td on t (d); load stats 's/explain_indexmerge_stats_t.json'; explain format = 'brief' select * from t where a < 50 or b < 50; id estRows task access object operator info -TableReader 98.00 root data:Selection -└─Selection 98.00 cop[tikv] or(lt(test.t.a, 50), lt(test.t.b, 50)) - └─TableFullScan 5000000.00 cop[tikv] table:t keep order:false +IndexMerge 98.00 root +├─TableRangeScan(Build) 49.00 cop[tikv] table:t range:[-inf,50), keep order:false +├─IndexRangeScan(Build) 49.00 cop[tikv] table:t, index:tb(b) range:[-inf,50), keep order:false +└─TableRowIDScan(Probe) 98.00 cop[tikv] table:t keep order:false explain format = 'brief' select * from t where (a < 50 or b < 50) and f > 100; id estRows task access object operator info -TableReader 98.00 root data:Selection -└─Selection 98.00 cop[tikv] gt(test.t.f, 100), or(lt(test.t.a, 50), lt(test.t.b, 50)) - └─TableFullScan 5000000.00 cop[tikv] table:t keep order:false +IndexMerge 98.00 root +├─TableRangeScan(Build) 49.00 cop[tikv] table:t range:[-inf,50), keep order:false +├─IndexRangeScan(Build) 49.00 cop[tikv] table:t, index:tb(b) range:[-inf,50), keep order:false +└─Selection(Probe) 98.00 cop[tikv] gt(test.t.f, 100) + └─TableRowIDScan 98.00 cop[tikv] table:t keep order:false explain format = 'brief' select * from t where b < 50 or c < 50; id estRows task access object operator info -TableReader 98.00 root data:Selection -└─Selection 98.00 cop[tikv] or(lt(test.t.b, 50), lt(test.t.c, 50)) - └─TableFullScan 5000000.00 cop[tikv] table:t keep order:false +IndexMerge 98.00 root +├─IndexRangeScan(Build) 49.00 cop[tikv] table:t, index:tb(b) range:[-inf,50), keep order:false +├─IndexRangeScan(Build) 49.00 cop[tikv] table:t, index:tc(c) range:[-inf,50), keep order:false +└─TableRowIDScan(Probe) 98.00 cop[tikv] table:t keep order:false set session tidb_enable_index_merge = on; explain format = 'brief' select * from t where a < 50 or b < 50; id estRows task access object operator info diff --git a/planner/core/integration_test.go b/planner/core/integration_test.go index 38048d4d30009..30c5a205ab257 100644 --- a/planner/core/integration_test.go +++ b/planner/core/integration_test.go @@ -2203,6 +2203,10 @@ func (s *testIntegrationSuite) TestOptimizeHintOnPartitionTable(c *C) { partition p1 values less than(11), partition p2 values less than(16));`) tk.MustExec(`insert into t values (1,1,"1"), (2,2,"2"), (8,8,"8"), (11,11,"11"), (15,15,"15")`) + tk.MustExec("set @@tidb_enable_index_merge = off") + defer func() { + tk.MustExec("set @@tidb_enable_index_merge = on") + }() // Create virtual tiflash replica info. dom := domain.GetDomain(tk.Se) diff --git a/planner/core/testdata/plan_suite_out.json b/planner/core/testdata/plan_suite_out.json index f70d1eb7e5550..013d86e227342 100644 --- a/planner/core/testdata/plan_suite_out.json +++ b/planner/core/testdata/plan_suite_out.json @@ -286,9 +286,9 @@ }, { "SQL": "select /*+ USE_INDEX_MERGE(t1, c_d_e, f_g) */ * from t where c < 1 or f > 2", - "Best": "TableReader(Table(t)->Sel([or(lt(test.t.c, 1), gt(test.t.f, 2))]))", + "Best": "IndexMergeReader(PartialPlans->[Index(t.c_d_e)[[-inf,1)], Index(t.f)[(2,+inf]]], TablePlan->Table(t))", "HasWarn": true, - "Hints": "use_index(@`sel_1` `test`.`t` )" + "Hints": "use_index_merge(@`sel_1` `t` `c_d_e`, `f`)" }, { "SQL": "select /*+ NO_INDEX_MERGE(), USE_INDEX_MERGE(t, primary, f_g, c_d_e) */ * from t where a < 1 or f > 2", @@ -304,15 +304,15 @@ }, { "SQL": "select /*+ USE_INDEX_MERGE(db2.t) */ * from t where c < 1 or f > 2", - "Best": "TableReader(Table(t)->Sel([or(lt(test.t.c, 1), gt(test.t.f, 2))]))", + "Best": "IndexMergeReader(PartialPlans->[Index(t.c_d_e)[[-inf,1)], Index(t.f)[(2,+inf]]], TablePlan->Table(t))", "HasWarn": true, - "Hints": "use_index(@`sel_1` `test`.`t` )" + "Hints": "use_index_merge(@`sel_1` `t` `c_d_e`, `f`)" }, { "SQL": "select /*+ USE_INDEX_MERGE(db2.t, c_d_e, f_g) */ * from t where c < 1 or f > 2", - "Best": "TableReader(Table(t)->Sel([or(lt(test.t.c, 1), gt(test.t.f, 2))]))", + "Best": "IndexMergeReader(PartialPlans->[Index(t.c_d_e)[[-inf,1)], Index(t.f)[(2,+inf]]], TablePlan->Table(t))", "HasWarn": true, - "Hints": "use_index(@`sel_1` `test`.`t` )" + "Hints": "use_index_merge(@`sel_1` `t` `c_d_e`, `f`)" } ] }, diff --git a/session/bootstrap.go b/session/bootstrap.go index e5c588ec5af01..1fdb93b58ff0e 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -541,11 +541,14 @@ const ( // version80 fixes the issue https://github.com/pingcap/tidb/issues/25422. // If the TiDB upgrading from the 4.x to a newer version, we keep the tidb_analyze_version to 1. version80 = 80 + // version81 insert "tidb_enable_index_merge|off" to mysql.GLOBAL_VARIABLES if there is no tidb_enable_index_merge. + // This will only happens when we upgrade a cluster before 4.0.0 to 4.0.0+. + version81 = 81 ) // currentBootstrapVersion is defined as a variable, so we can modify its value for testing. // please make sure this is the largest version -var currentBootstrapVersion int64 = version80 +var currentBootstrapVersion int64 = version81 var ( bootstrapVersion = []func(Session, int64){ @@ -629,6 +632,7 @@ var ( upgradeToVer78, upgradeToVer79, upgradeToVer80, + upgradeToVer81, } ) @@ -1655,6 +1659,29 @@ func upgradeToVer80(s Session, ver int64) { mysql.SystemDB, mysql.GlobalVariablesTable, variable.TiDBAnalyzeVersion, 1) } +// For users that upgrade TiDB from a pre-4.0 version, we want to disable index merge by default. +// This helps minimize query plan regressions. +func upgradeToVer81(s Session, ver int64) { + if ver >= version81 { + return + } + // Check if tidb_enable_index_merge exists in mysql.GLOBAL_VARIABLES. + // If not, insert "tidb_enable_index_merge | off". + ctx := context.Background() + rs, err := s.ExecuteInternal(ctx, "SELECT VARIABLE_VALUE FROM %n.%n WHERE VARIABLE_NAME=%?;", + mysql.SystemDB, mysql.GlobalVariablesTable, variable.TiDBEnableIndexMerge) + terror.MustNil(err) + req := rs.NewChunk(nil) + err = rs.Next(ctx, req) + terror.MustNil(err) + if req.NumRows() != 0 { + return + } + + mustExecute(s, "INSERT HIGH_PRIORITY IGNORE INTO %n.%n VALUES (%?, %?);", + mysql.SystemDB, mysql.GlobalVariablesTable, variable.TiDBEnableIndexMerge, variable.Off) +} + func writeOOMAction(s Session) { comment := "oom-action is `log` by default in v3.0.x, `cancel` by default in v4.0.11+" mustExecute(s, `INSERT HIGH_PRIORITY INTO %n.%n VALUES (%?, %?, %?) ON DUPLICATE KEY UPDATE VARIABLE_VALUE= %?`, diff --git a/session/bootstrap_test.go b/session/bootstrap_test.go index 6caf7e702c10b..914d516912ea1 100644 --- a/session/bootstrap_test.go +++ b/session/bootstrap_test.go @@ -928,3 +928,144 @@ func TestAnalyzeVersionUpgradeFrom300To500(t *testing.T) { require.Equal(t, 1, row.Len()) require.Equal(t, "1", row.GetString(0)) } + +func TestIndexMergeInNewCluster(t *testing.T) { + store, err := mockstore.NewMockStore() + require.NoError(t, err) + // Indicates we are in a new cluster. + require.Equal(t, int64(notBootstrapped), getStoreBootstrapVersion(store)) + dom, err := BootstrapSession(store) + require.NoError(t, err) + defer func() { require.NoError(t, store.Close()) }() + defer dom.Close() + se := createSessionAndSetID(t, store) + + // In a new created cluster(above 5.4+), tidb_enable_index_merge is 1 by default. + mustExec(t, se, "use test;") + r := mustExec(t, se, "select @@tidb_enable_index_merge;") + require.NotNil(t, r) + + ctx := context.Background() + chk := r.NewChunk(nil) + err = r.Next(ctx, chk) + require.NoError(t, err) + require.Equal(t, 1, chk.NumRows()) + row := chk.GetRow(0) + require.Equal(t, 1, row.Len()) + require.Equal(t, int64(1), row.GetInt64(0)) +} + +func TestIndexMergeUpgradeFrom300To540(t *testing.T) { + ctx := context.Background() + store, _ := createStoreAndBootstrap(t) + defer func() { require.NoError(t, store.Close()) }() + + // Upgrade from 3.0.0 to 5.4+. + ver300 := 33 + seV3 := createSessionAndSetID(t, store) + txn, err := store.Begin() + require.NoError(t, err) + m := meta.NewMeta(txn) + err = m.FinishBootstrap(int64(ver300)) + require.NoError(t, err) + err = txn.Commit(context.Background()) + require.NoError(t, err) + mustExec(t, seV3, fmt.Sprintf("update mysql.tidb set variable_value=%d where variable_name='tidb_server_version'", ver300)) + mustExec(t, seV3, fmt.Sprintf("delete from mysql.GLOBAL_VARIABLES where variable_name='%s'", variable.TiDBEnableIndexMerge)) + mustExec(t, seV3, "commit") + unsetStoreBootstrapped(store.UUID()) + ver, err := getBootstrapVersion(seV3) + require.NoError(t, err) + require.Equal(t, int64(ver300), ver) + + // We are now in 3.0.0, check tidb_enable_index_merge shoudle not exist. + res := mustExec(t, seV3, fmt.Sprintf("select * from mysql.GLOBAL_VARIABLES where variable_name='%s'", variable.TiDBEnableIndexMerge)) + chk := res.NewChunk(nil) + err = res.Next(ctx, chk) + require.NoError(t, err) + require.Equal(t, 0, chk.NumRows()) + + domCurVer, err := BootstrapSession(store) + require.NoError(t, err) + defer domCurVer.Close() + seCurVer := createSessionAndSetID(t, store) + ver, err = getBootstrapVersion(seCurVer) + require.NoError(t, err) + require.Equal(t, currentBootstrapVersion, ver) + + // We are now in 5.x, tidb_enable_index_merge should be off. + res = mustExec(t, seCurVer, "select @@tidb_enable_index_merge") + chk = res.NewChunk(nil) + err = res.Next(ctx, chk) + require.NoError(t, err) + require.Equal(t, 1, chk.NumRows()) + row := chk.GetRow(0) + require.Equal(t, 1, row.Len()) + require.Equal(t, int64(0), row.GetInt64(0)) +} + +func TestIndexMergeUpgradeFrom400To540(t *testing.T) { + for i := 0; i < 2; i++ { + ctx := context.Background() + store, _ := createStoreAndBootstrap(t) + defer func() { require.NoError(t, store.Close()) }() + + // upgrade from 4.0.0 to 5.4+. + ver400 := 46 + seV4 := createSessionAndSetID(t, store) + txn, err := store.Begin() + require.NoError(t, err) + m := meta.NewMeta(txn) + err = m.FinishBootstrap(int64(ver400)) + require.NoError(t, err) + err = txn.Commit(context.Background()) + require.NoError(t, err) + mustExec(t, seV4, fmt.Sprintf("update mysql.tidb set variable_value=%d where variable_name='tidb_server_version'", ver400)) + mustExec(t, seV4, fmt.Sprintf("update mysql.GLOBAL_VARIABLES set variable_value='%s' where variable_name='%s'", variable.Off, variable.TiDBEnableIndexMerge)) + mustExec(t, seV4, "commit") + unsetStoreBootstrapped(store.UUID()) + ver, err := getBootstrapVersion(seV4) + require.NoError(t, err) + require.Equal(t, int64(ver400), ver) + + // We are now in 4.0.0, tidb_enable_index_merge is off. + res := mustExec(t, seV4, fmt.Sprintf("select * from mysql.GLOBAL_VARIABLES where variable_name='%s'", variable.TiDBEnableIndexMerge)) + chk := res.NewChunk(nil) + err = res.Next(ctx, chk) + require.NoError(t, err) + require.Equal(t, 1, chk.NumRows()) + row := chk.GetRow(0) + require.Equal(t, 2, row.Len()) + require.Equal(t, variable.Off, row.GetString(1)) + + if i == 0 { + // For the first time, We set tidb_enable_index_merge as on. + // And after upgrade to 5.x, tidb_enable_index_merge should remains to be on. + // For the second it should be off. + mustExec(t, seV4, "set global tidb_enable_index_merge = on") + } + + // Upgrade to 5.x. + domCurVer, err := BootstrapSession(store) + require.NoError(t, err) + defer domCurVer.Close() + seCurVer := createSessionAndSetID(t, store) + ver, err = getBootstrapVersion(seCurVer) + require.NoError(t, err) + require.Equal(t, currentBootstrapVersion, ver) + + // We are now in 5.x, tidb_enable_index_merge should be on because we enable it in 4.0.0. + res = mustExec(t, seCurVer, "select @@tidb_enable_index_merge") + chk = res.NewChunk(nil) + err = res.Next(ctx, chk) + require.NoError(t, err) + require.Equal(t, 1, chk.NumRows()) + row = chk.GetRow(0) + require.Equal(t, 1, row.Len()) + if i == 0 { + require.Equal(t, int64(1), row.GetInt64(0)) + } else { + require.Equal(t, int64(0), row.GetInt64(0)) + } + } +} diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 3b0c8f33402e9..a2136587a403d 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -1172,7 +1172,7 @@ func NewSessionVars() *SessionVars { SlowQueryFile: config.GetGlobalConfig().Log.SlowQueryFile, WaitSplitRegionFinish: DefTiDBWaitSplitRegionFinish, WaitSplitRegionTimeout: DefWaitSplitRegionTimeout, - enableIndexMerge: false, + enableIndexMerge: DefTiDBEnableIndexMerge, NoopFuncsMode: TiDBOptOnOffWarn(DefTiDBEnableNoopFuncs), replicaRead: kv.ReplicaReadLeader, AllowRemoveAutoInc: DefTiDBAllowRemoveAutoInc, diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 3491f28bc73dc..396c93bddba1b 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -537,7 +537,7 @@ var defaultSysVars = []*SysVar{ s.SetEnableCascadesPlanner(TiDBOptOn(val)) return nil }}, - {Scope: ScopeGlobal | ScopeSession, Name: TiDBEnableIndexMerge, Value: Off, Type: TypeBool, SetSession: func(s *SessionVars, val string) error { + {Scope: ScopeGlobal | ScopeSession, Name: TiDBEnableIndexMerge, Value: BoolToOnOff(DefTiDBEnableIndexMerge), Type: TypeBool, SetSession: func(s *SessionVars, val string) error { s.SetEnableIndexMerge(TiDBOptOn(val)) return nil }}, diff --git a/sessionctx/variable/sysvar_test.go b/sessionctx/variable/sysvar_test.go index e15c9f92b92b8..91b3451f15ed9 100644 --- a/sessionctx/variable/sysvar_test.go +++ b/sessionctx/variable/sysvar_test.go @@ -825,3 +825,12 @@ func TestDefaultCharsetAndCollation(t *testing.T) { require.NoError(t, err) require.Equal(t, val, mysql.DefaultCollationName) } + +func TestIndexMergeSwitcher(t *testing.T) { + vars := NewSessionVars() + vars.GlobalVarsAccessor = NewMockGlobalAccessor4Tests() + val, err := GetSessionOrGlobalSystemVar(vars, TiDBEnableIndexMerge) + require.NoError(t, err) + require.Equal(t, DefTiDBEnableIndexMerge, true) + require.Equal(t, BoolToOnOff(DefTiDBEnableIndexMerge), val) +} diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index ccec9e1b5a8fc..7540000f4fc8b 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -779,6 +779,7 @@ const ( DefTiDBRegardNULLAsPoint = true DefEnablePlacementCheck = true DefTimestamp = "0" + DefTiDBEnableIndexMerge = true ) // Process global variables.