From af5934d2642ba26f5a7a82ac60e9d1e3fa9d07ce Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Fri, 12 Mar 2021 11:33:00 +0800 Subject: [PATCH 1/2] Revert "executor: open childExec during execution for UnionExec (#21561)" This reverts commit 0eb8ff97bc1177fd9ac62f2d925eeec196211036. --- executor/executor.go | 43 ++++----------------------------------- executor/executor_test.go | 34 ------------------------------- 2 files changed, 4 insertions(+), 73 deletions(-) diff --git a/executor/executor.go b/executor/executor.go index 3303436c39a44..201ef2e248f56 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -28,7 +28,6 @@ import ( "github.com/cznic/mathutil" "github.com/opentracing/opentracing-go" "github.com/pingcap/errors" - "github.com/pingcap/failpoint" "github.com/pingcap/parser/ast" "github.com/pingcap/parser/auth" "github.com/pingcap/parser/model" @@ -1442,12 +1441,6 @@ type UnionExec struct { results []*chunk.Chunk wg sync.WaitGroup initialized bool - mu struct { - *sync.Mutex - maxOpenedChildID int - } - - childInFlightForTest int32 } // unionWorkerResult stores the result for a union worker. @@ -1467,11 +1460,12 @@ func (e *UnionExec) waitAllFinished() { // Open implements the Executor Open interface. func (e *UnionExec) Open(ctx context.Context) error { + if err := e.baseExecutor.Open(ctx); err != nil { + return err + } e.stopFetchData.Store(false) e.initialized = false e.finished = make(chan struct{}) - e.mu.Mutex = &sync.Mutex{} - e.mu.maxOpenedChildID = -1 return nil } @@ -1517,19 +1511,6 @@ func (e *UnionExec) resultPuller(ctx context.Context, workerID int) { e.wg.Done() }() for childID := range e.childIDChan { - e.mu.Lock() - if childID > e.mu.maxOpenedChildID { - e.mu.maxOpenedChildID = childID - } - e.mu.Unlock() - if err := e.children[childID].Open(ctx); err != nil { - result.err = err - e.stopFetchData.Store(true) - e.resultPool <- result - } - failpoint.Inject("issue21441", func() { - atomic.AddInt32(&e.childInFlightForTest, 1) - }) for { if e.stopFetchData.Load().(bool) { return @@ -1544,20 +1525,12 @@ func (e *UnionExec) resultPuller(ctx context.Context, workerID int) { e.resourcePools[workerID] <- result.chk break } - failpoint.Inject("issue21441", func() { - if int(atomic.LoadInt32(&e.childInFlightForTest)) > e.concurrency { - panic("the count of child in flight is larger than e.concurrency unexpectedly") - } - }) e.resultPool <- result if result.err != nil { e.stopFetchData.Store(true) return } } - failpoint.Inject("issue21441", func() { - atomic.AddInt32(&e.childInFlightForTest, -1) - }) } } @@ -1596,15 +1569,7 @@ func (e *UnionExec) Close() error { for range e.childIDChan { } } - // We do not need to acquire the e.mu.Lock since all the resultPuller can be - // promised to exit when reaching here (e.childIDChan been closed). - var firstErr error - for i := 0; i <= e.mu.maxOpenedChildID; i++ { - if err := e.children[i].Close(); err != nil && firstErr == nil { - firstErr = err - } - } - return firstErr + return e.baseExecutor.Close() } // ResetContextOfStmt resets the StmtContext and session variables. diff --git a/executor/executor_test.go b/executor/executor_test.go index 1b90e1f79375a..aff376228b938 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -7399,40 +7399,6 @@ func (s *testSuite) TestOOMActionPriority(c *C) { c.Assert(action.GetPriority(), Equals, int64(memory.DefLogPriority)) } -func (s *testSerialSuite) TestIssue21441(c *C) { - err := failpoint.Enable("github.com/pingcap/tidb/executor/issue21441", `return`) - c.Assert(err, IsNil) - defer func() { - err := failpoint.Disable("github.com/pingcap/tidb/executor/issue21441") - c.Assert(err, IsNil) - }() - - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("use test") - tk.MustExec("drop table if exists t") - tk.MustExec("create table t(a int)") - tk.MustExec(`insert into t values(1),(2),(3)`) - tk.Se.GetSessionVars().InitChunkSize = 1 - tk.Se.GetSessionVars().MaxChunkSize = 1 - sql := ` -select a from t union all -select a from t union all -select a from t union all -select a from t union all -select a from t union all -select a from t union all -select a from t union all -select a from t` - tk.MustQuery(sql).Sort().Check(testkit.Rows( - "1", "1", "1", "1", "1", "1", "1", "1", - "2", "2", "2", "2", "2", "2", "2", "2", - "3", "3", "3", "3", "3", "3", "3", "3", - )) - - tk.MustQuery("select a from (" + sql + ") t order by a limit 4").Check(testkit.Rows("1", "1", "1", "1")) - tk.MustQuery("select a from (" + sql + ") t order by a limit 7, 4").Check(testkit.Rows("1", "2", "2", "2")) -} - func (s *testSuite) Test17780(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") From 48f2027e42e6b84ae9721adac166fc3b56a4e6d6 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Thu, 18 Mar 2021 15:25:57 +0800 Subject: [PATCH 2/2] *: hide the config and session var --- config/config.go | 2 +- config/config.toml.example | 2 -- executor/show_test.go | 11 +++++++++++ sessionctx/variable/tidb_vars.go | 1 + 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index 8e1a4fe1d4214..923a3cba0a7ec 100644 --- a/config/config.go +++ b/config/config.go @@ -534,7 +534,7 @@ type Experimental struct { // Whether enable creating expression index. AllowsExpressionIndex bool `toml:"allow-expression-index" json:"allow-expression-index"` // Whether enable global kill. - EnableGlobalKill bool `toml:"enable-global-kill" json:"enable-global-kill"` + EnableGlobalKill bool `toml:"enable-global-kill" json:"-"` } var defTiKVCfg = tikvcfg.DefaultConfig() diff --git a/config/config.toml.example b/config/config.toml.example index 60e86c847f205..975c095b53671 100644 --- a/config/config.toml.example +++ b/config/config.toml.example @@ -468,8 +468,6 @@ history-size = 24 [experimental] # enable creating expression index. allow-expression-index = false -# enable global kill. -enable-global-kill = false # server level isolation read by engines and labels [isolation-read] diff --git a/executor/show_test.go b/executor/show_test.go index 600c47c7c0586..ecc858cbaf2dd 100644 --- a/executor/show_test.go +++ b/executor/show_test.go @@ -1133,6 +1133,17 @@ func (s *testSuite5) TestInvisibleCoprCacheConfig(c *C) { c.Assert(strings.Contains(configValue, coprCacheVal), Equals, true) } +func (s *testSuite5) TestInvisibleGlobalKillConfig(c *C) { + se1, err := session.CreateSession(s.store) + c.Assert(err, IsNil) + tk := testkit.NewTestKitWithSession(c, s.store, se1) + rows := tk.MustQuery("show variables like '%config%'").Rows() + c.Assert(len(rows), Equals, 1) + configValue := rows[0][1].(string) + globalKillVal := "global-kill" + c.Assert(strings.Contains(configValue, globalKillVal), Equals, false) +} + func (s *testSerialSuite1) TestShowCreateTableWithIntegerDisplayLengthWarnings(c *C) { parsertypes.TiDBStrictIntegerDisplayWidth = true defer func() { diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index 8acc5841bd1a6..dcedbd56affc6 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -714,6 +714,7 @@ var FeatureSwitchVariables = []string{ TiDBPartitionPruneMode, TiDBIntPrimaryKeyDefaultAsClustered, TiDBEnableExtendedStats, + TiDBEnableIndexMergeJoin, } // FilterImplicitFeatureSwitch is used to filter result of show variables, these switches should be turn blind to users.