pingcap / tidb

TiDB is an open-source, cloud-native, distributed, MySQL-Compatible database for elastic scale and real-time analytics. Try AI-powered Chat2Query free at : https://www.pingcap.com/tidb-serverless/
https://pingcap.com
Apache License 2.0
37.07k stars 5.83k forks source link

Reconsider process info implementation for statement context reuse #26161

Open tiancaiamao opened 3 years ago

tiancaiamao commented 3 years ago

Enhancement

To reduce object allocation in the point get, I aggressively reuse the objects. When I try to reuse the StatementContext in https://github.com/pingcap/tidb/pull/26007, I found some problem.

In the past, SetProcessInfo() get a snapshot of the process info:

    pi := util.ProcessInfo{
        ID:               s.sessionVars.ConnectionID,
        Port:             s.sessionVars.Port,
        DB:               s.sessionVars.CurrentDB,
        Command:          command,
        Plan:             p,
        PlanExplainRows:  plannercore.GetExplainRowsForPlan(p),
        RuntimeStatsColl: s.sessionVars.StmtCtx.RuntimeStatsColl,
        Time:             t,
        State:            s.Status(),
        Info:             sql,
        CurTxnStartTS:    curTxnStartTS,
        StmtCtx:          s.sessionVars.StmtCtx,
        StatsInfo:        plannercore.GetStatsInfo,
        MaxExecutionTime: maxExecutionTime,
        RedactSQL:        s.sessionVars.EnableRedactLog,
    }
    s.processInfo.Store(&pi)

This is a shallow copy, when ResetContextStmt() create objects, the pi referred objects are never touched again. But if I reuse those object, the pi will be modified too! It's not a real snapshot any more.

So we can not reuse the old StmtCtx is the a new statement .... the pi is still referencing it.

xhebox commented 3 years ago

Hmm, I think the PR could be reopened after solving the issue. My first idea is to provide a Clone for StmtCtx, and remove RuntimeStatsColl in the processinfo.