pingcap / tidb

TiDB - the open-source, cloud-native, distributed SQL database designed for modern applications.
https://pingcap.com
Apache License 2.0
37.28k stars 5.84k forks source link

Add a real session to `CollectRemoteDuplicateRows` #46737

Open zimulala opened 1 year ago

zimulala commented 1 year ago

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

Run TestConcurrentDDLCreateUniqueIndex.

https://github.com/pingcap/tidb/blob/09a83b8a46cc53db68d4a79ff4b51e95cfa6e031/ddl/ingest/backend.go#L137

https://github.com/pingcap/tidb/blob/09a83b8a46cc53db68d4a79ff4b51e95cfa6e031/br/pkg/lightning/backend/kv/kv2sql.go#L124-L136

And we do RewriteAstExpr in CollectGeneratedColumns, we'd better to a real session to CollectRemoteDuplicateRows in FinishImport.

2. What did you expect to see? (Required)

no panic

3. What did you see instead (Required)

"invalid memory address or nil pointer dereference\""] [stack="github.com/pingcap/tidb/util.Recover
    util/misc.go:120
runtime.gopanic
    GOROOT/src/runtime/panic.go:914
runtime.panicmem
    GOROOT/src/runtime/panic.go:261
runtime.sigpanic
    GOROOT/src/runtime/signal_unix.go:861
github.com/pingcap/tidb/sessionctx/variable.(*SessionVars).GetSessionOrGlobalSystemVar
    sessionctx/variable/session.go:2469
github.com/pingcap/tidb/planner/core.(*expressionRewriter).adjustUTF8MB4Collation
    planner/core/expression_rewriter.go:1209
github.com/pingcap/tidb/planner/core.(*expressionRewriter).Leave
    planner/core/expression_rewriter.go:1241
github.com/pingcap/tidb/types/parser_driver.(*ValueExpr).Accept
    types/parser_driver/value_expr.go:233
github.com/pingcap/tidb/parser/ast.(*FuncCallExpr).Accept
    parser/ast/functions.go:578
github.com/pingcap/tidb/planner/core.(*PlanBuilder).rewriteExprNode
    planner/core/expression_rewriter.go:201
github.com/pingcap/tidb/planner/core.(*PlanBuilder).rewriteWithPreprocess
    planner/core/expression_rewriter.go:146
github.com/pingcap/tidb/planner/core.(*PlanBuilder).rewrite
    planner/core/expression_rewriter.go:114
github.com/pingcap/tidb/planner/core.rewriteAstExpr
    planner/core/expression_rewriter.go:80
github.com/pingcap/tidb/br/pkg/lightning/backend/kv.CollectGeneratedColumns
    br/pkg/lightning/backend/kv/sql2kv.go:118
github.com/pingcap/tidb/br/pkg/lightning/backend/kv.NewTableKVDecoder
    br/pkg/lightning/backend/kv/kv2sql.go:136
github.com/pingcap/tidb/br/pkg/lightning/backend/local.NewDupeDetector
    br/pkg/lightning/backend/local/duplicate.go:441
github.com/pingcap/tidb/br/pkg/lightning/backend/local.(*DupeController).CollectRemoteDuplicateRows
    br/pkg/lightning/backend/local/duplicate.go:987
github.com/pingcap/tidb/ddl/ingest.(*litBackendCtx).FinishImport
    ddl/ingest/backend.go:137
github.com/pingcap/tidb/ddl.runIngestReorgJob
    ddl/index.go:940
github.com/pingcap/tidb/ddl.doReorgWorkForCreateIndex
    ddl/index.go:869
github.com/pingcap/tidb/ddl.(*worker).onCreateIndex
    ddl/index.go:685
github.com/pingcap/tidb/ddl.(*worker).runDDLJob
    ddl/ddl_worker.go:1066
github.com/pingcap/tidb/ddl.(*worker).HandleDDLJobTable
    ddl/ddl_worker.go:786
github.com/pingcap/tidb/ddl.(*ddl).delivery2worker.func1
    ddl/job_table.go:414
github.com/pingcap/tidb/util.(*WaitGroupWrapper).Run.func1
    util/wait_group_wrapper.go:154"] [stack="github.com/pingcap/tidb/util.Recover
    util/misc.go:116
runtime.gopanic

https://do.pingcap.net/jenkins/blue/organizations/jenkins/pingcap%2Ftidb%2Fghpr_check2/detail/ghpr_check2/28159/pipeline

4. What is your TiDB version? (Required)

https://github.com/pingcap/tidb/pull/46667 https://github.com/pingcap/tidb/pull/46667/commits/30e9373044a6a87e3f6e8e155f3eabdd8902abe4

lance6716 commented 1 year ago

One problem is that these's no "real session" when use lightning as a standalone binary. In the old way we just extend the mocked session that lightning uses.

It's better that when TiDB uses lightning as a library we can use the real session and fallback to mocked session otherwise. And also it can solve many other problems.

cc @D3Hunter