some inTest assert panic comes from ddl/systable/min_job_id when testing #54868
Labels
affects-7.5
affects-8.1
component/ddl
This issue is related to DDL of TiDB.
impact/panic
severity/minor
type/bug
The issue is confirmed as a bug.
Comments
AilinKid
added
type/bug
|
apply this diff on 8.1.0, run any case that create domain&store, it will report same error, the cause is after line 3321 there will be Lines 3321 to 3332 in 945d07c this issue became obvious as diff --git a/pkg/ddl/job_table.go b/pkg/ddl/job_table.go
index aa2ebc3d51..41884214bb 100644
--- a/pkg/ddl/job_table.go
+++ b/pkg/ddl/job_table.go
@@ -97,6 +97,10 @@ func (d *ddl) getJob(se *sess.Session, tp jobType, filter func(*model.Job) (bool
excludedJobIDs = fmt.Sprintf("and job_id not in (%s)", ids)
}
sql := fmt.Sprintf(getJobSQL, not, excludedJobIDs)
+ if intest.COUNTER.Load() == 1 {
+ se.Context.GetSessionVars().InsideDDLGet.Store(true)
+ defer se.Context.GetSessionVars().InsideDDLGet.Store(false)
+ }
rows, err := se.Execute(context.Background(), sql, label)
if err != nil {
return nil, errors.Trace(err)
diff --git a/pkg/executor/executor.go b/pkg/executor/executor.go
index 71feeb2086..e01e5f08b8 100644
--- a/pkg/executor/executor.go
+++ b/pkg/executor/executor.go
@@ -1990,6 +1990,10 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) {
sc = vars.InitStatementContext()
}
sc.SetTimeZone(vars.Location())
+ if intest.COUNTER.Load() == 1 && vars.InsideDDLGet.Load() {
+ <-intest.SYNCCH
+ intest.COUNTER.Add(1)
+ }
sc.TaskID = stmtctx.AllocateTaskID()
sc.CTEStorageMap = map[int]*CTEStorages{}
sc.IsStaleness = false
diff --git a/pkg/expression/context/context.go b/pkg/expression/context/context.go
index 73a817fdf4..2224dce436 100644
--- a/pkg/expression/context/context.go
+++ b/pkg/expression/context/context.go
@@ -16,6 +16,8 @@ package context
import (
"fmt"
+ "runtime"
+ "strings"
"time"
"github.com/pingcap/tidb/pkg/errctx"
@@ -23,7 +25,10 @@ import (
"github.com/pingcap/tidb/pkg/sessionctx/stmtctx"
"github.com/pingcap/tidb/pkg/sessionctx/variable"
"github.com/pingcap/tidb/pkg/types"
+ "github.com/pingcap/tidb/pkg/util/intest"
+ "github.com/pingcap/tidb/pkg/util/logutil"
"github.com/pingcap/tidb/pkg/util/mathutil"
+ "go.uber.org/zap"
)
// EvalContext is used to evaluate an expression
@@ -104,3 +109,24 @@ type ExprContext interface {
// GetGroupConcatMaxLen returns the value of the 'group_concat_max_len' system variable.
GetGroupConcatMaxLen() uint64
}
+
+// AssertLocationWithSessionVars asserts the location in the context and session variables are the same.
+// It is only used for testing.
+func AssertLocationWithSessionVars(ctxLoc *time.Location, vars *variable.SessionVars) {
+ if intest.COUNTER.Load() == 2 {
+ buf := make([]byte, 1<<16)
+ runtime.Stack(buf, true)
+ if strings.Contains(string(buf), "getGeneralJob") {
+ <-intest.SYNCCH
+ }
+ }
+ ctxLocStr := ctxLoc.String()
+ varsLocStr := vars.Location().String()
+ stmtLocStr := vars.StmtCtx.TimeZone().String()
+ logutil.BgLogger().Info("AssertLocationWithSessionVars", zap.String("ctxLoc", ctxLocStr),
+ zap.String("varsLoc", varsLocStr), zap.String("stmtLoc", stmtLocStr))
+ intest.Assert(ctxLocStr == varsLocStr && ctxLocStr == stmtLocStr,
+ "location mismatch, ctxLoc: %s, varsLoc: %s, stmtLoc: %s",
+ ctxLoc.String(), ctxLocStr, stmtLocStr,
+ )
+}
diff --git a/pkg/expression/contextimpl/sessionctx.go b/pkg/expression/contextimpl/sessionctx.go
index 5d792f2c9b..c6cfc03c28 100644
--- a/pkg/expression/contextimpl/sessionctx.go
+++ b/pkg/expression/contextimpl/sessionctx.go
@@ -167,6 +167,10 @@ func (ctx *SessionEvalContext) SQLMode() mysql.SQLMode {
// TypeCtx returns the types.Context
func (ctx *SessionEvalContext) TypeCtx() types.Context {
+ tc := ctx.sctx.GetSessionVars().StmtCtx.TypeCtx()
+ if intest.InTest {
+ exprctx.AssertLocationWithSessionVars(tc.Location(), ctx.sctx.GetSessionVars())
+ }
return ctx.sctx.GetSessionVars().StmtCtx.TypeCtx()
}
diff --git a/pkg/session/session.go b/pkg/session/session.go
index 2cc9d07167..a811343f9d 100644
--- a/pkg/session/session.go
+++ b/pkg/session/session.go
@@ -3329,7 +3329,14 @@ func bootstrapSessionImpl(store kv.Storage, createSessionsImpl func(store kv.Sto
if err != nil {
return nil, err
}
+ intest.COUNTER.Add(1)
+ intest.SYNCCH <- struct{}{}
+ systemTZ, err := timeutil.GetSystemTZ()
+ logutil.BgLogger().Warn("before set timezone", zap.String("tz", systemTZ), zap.Error(err))
timeutil.SetSystemTZ(tz)
+ systemTZ, err = timeutil.GetSystemTZ()
+ intest.SYNCCH <- struct{}{}
+ logutil.BgLogger().Warn("after set timezone", zap.String("tz", systemTZ), zap.Error(err))
// get the flag from `mysql`.`tidb` which indicating if new collations are enabled.
newCollationEnabled, err := loadCollationParameter(ctx, ses[0])
diff --git a/pkg/sessionctx/variable/session.go b/pkg/sessionctx/variable/session.go
index c4bb81ee8d..7cb8a27fdb 100644
--- a/pkg/sessionctx/variable/session.go
+++ b/pkg/sessionctx/variable/session.go
@@ -788,8 +788,8 @@ type SessionVars struct {
LastFoundRows uint64
// StmtCtx holds variables for current executing statement.
- StmtCtx *stmtctx.StatementContext
-
+ StmtCtx *stmtctx.StatementContext
+ InsideDDLGet atomic.Bool
// RefCountOfStmtCtx indicates the reference count of StmtCtx. When the
// StmtCtx is accessed by other sessions, e.g. oom-alarm-handler/expensive-query-handler, add one first.
// Note: this variable should be accessed and updated by atomic operations.
diff --git a/pkg/util/intest/common.go b/pkg/util/intest/common.go
index a2ebccd4b3..4017451490 100644
--- a/pkg/util/intest/common.go
+++ b/pkg/util/intest/common.go
@@ -16,6 +16,8 @@
package intest
+import "sync/atomic"
+
// InTest checks if the code is running in test.
const InTest = false
@@ -34,3 +36,7 @@ func AssertNoError(_ error, _ ...any) {}
// AssertFunc is a stub function in release build.
// See the same function `util/intest/assert.go` for the real implement in test.
func AssertFunc(_ func() bool, _ ...any) {}
+
+var COUNTER atomic.Int32
+
+var SYNCCH = make(chan struct{})
diff --git a/pkg/util/intest/intest.go b/pkg/util/intest/intest.go
index a96d2fbf73..fd10044f16 100644
--- a/pkg/util/intest/intest.go
+++ b/pkg/util/intest/intest.go
@@ -16,5 +16,11 @@
package intest
+import "sync/atomic"
+
// InTest checks if the code is running in test.
const InTest = true
+
+var COUNTER atomic.Int32
+
+var SYNCCH = make(chan struct{}) |
|
7.1/7.5/8.1 have similar issue, but they don't have this assert, and it's harder to reproduce this problem, so mark it as minor |
13 tasks
D3Hunter
added
affects-7.1
affects-7.5
affects-8.1
and removed
affects-7.1
affects-7.5
affects-8.1
labels
hawkingrei
pushed a commit
to hawkingrei/tidb
that referenced
this issue
Aug 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug Report
Please answer these questions before submitting your issue. Thanks!
1. Minimal reproduce step (Required)
2. What did you expect to see? (Required)
no panic
3. What did you see instead (Required)
occasionally panic
4. What is your TiDB version? (Required)
master
The text was updated successfully, but these errors were encountered: