From 35eaf4d1886c348346b5b2176cbab9ebf8379536 Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Wed, 24 Jan 2024 15:56:05 +0800 Subject: [PATCH 1/3] fix update process info will data race with expensivequery check Signed-off-by: AilinKid <314806019@qq.com> --- pkg/session/session.go | 7 ++-- pkg/util/processinfo.go | 6 ++++ pkg/util/processinfo_test.go | 67 ++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 pkg/util/processinfo_test.go diff --git a/pkg/session/session.go b/pkg/session/session.go index 65b473e21a4a5..818a71f191f45 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -1558,9 +1558,12 @@ func (s *session) UpdateProcessInfo() { if pi == nil || pi.CurTxnStartTS != 0 { return } + // do not modify this two fields in place, see issue: issues/50607 + shallowCP := pi.Clone() // Update the current transaction start timestamp. - pi.CurTxnStartTS = s.sessionVars.TxnCtx.StartTS - pi.CurTxnCreateTime = s.sessionVars.TxnCtx.CreateTime + shallowCP.CurTxnStartTS = s.sessionVars.TxnCtx.StartTS + shallowCP.CurTxnCreateTime = s.sessionVars.TxnCtx.CreateTime + s.processInfo.Store(&shallowCP) } func (s *session) getOomAlarmVariablesInfo() util.OOMAlarmVariablesInfo { diff --git a/pkg/util/processinfo.go b/pkg/util/processinfo.go index 3107e095e0380..d85b9ca7620d1 100644 --- a/pkg/util/processinfo.go +++ b/pkg/util/processinfo.go @@ -73,6 +73,12 @@ type ProcessInfo struct { RedactSQL bool } +// Clone return a shallow clone copy of this processInfo. +func (pi *ProcessInfo) Clone() *ProcessInfo { + cp := *pi + return &cp +} + // ToRowForShow returns []interface{} for the row data of "SHOW [FULL] PROCESSLIST". func (pi *ProcessInfo) ToRowForShow(full bool) []interface{} { var info interface{} diff --git a/pkg/util/processinfo_test.go b/pkg/util/processinfo_test.go new file mode 100644 index 0000000000000..a877564071711 --- /dev/null +++ b/pkg/util/processinfo_test.go @@ -0,0 +1,67 @@ +// Copyright 2024 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "fmt" + "reflect" + "testing" + + "github.com/pingcap/tidb/pkg/sessionctx/stmtctx" + "github.com/pingcap/tidb/pkg/util/memory" + "github.com/stretchr/testify/require" +) + +func MyFunc(interface{}) map[string]uint64 { + return nil +} + +func TestProcessInfoShallowCP(t *testing.T) { + mem := memory.NewTracker(-1, -1) + mem.Consume(1<<30 + 1<<29 + 1<<28 + 1<<27) + + var refCount stmtctx.ReferenceCount = 0 + info := &ProcessInfo{ + ID: 233, + User: "PingCAP", + Host: "127.0.0.1", + DB: "Database", + Info: "select * from table where a > 1", + CurTxnStartTS: 23333, + StatsInfo: MyFunc, + StmtCtx: stmtctx.NewStmtCtx(), + RefCountOfStmtCtx: &refCount, + MemTracker: mem, + RedactSQL: false, + SessionAlias: "alias123", + } + + cp := info.Clone() + require.False(t, cp == info) + require.True(t, cp.ID == info.ID) + require.True(t, cp.User == info.User) + require.True(t, cp.Host == info.Host) + require.True(t, cp.DB == info.DB) + require.True(t, cp.Info == info.Info) + require.True(t, cp.CurTxnStartTS == info.CurTxnStartTS) + // reflect.DeepEqual couldn't cmp two function pointer. + require.True(t, reflect.ValueOf(cp.StatsInfo).Pointer() == reflect.ValueOf(info.StatsInfo).Pointer()) + fmt.Println(reflect.ValueOf(cp.StatsInfo).Pointer(), reflect.ValueOf(info.StatsInfo).Pointer()) + require.True(t, cp.StmtCtx == info.StmtCtx) + require.True(t, cp.RefCountOfStmtCtx == info.RefCountOfStmtCtx) + require.True(t, cp.MemTracker == info.MemTracker) + require.True(t, cp.RedactSQL == info.RedactSQL) + require.True(t, cp.SessionAlias == info.SessionAlias) +} From a3ff50d3fb0098b914d48ae93966beb1ad77bc1d Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Wed, 24 Jan 2024 16:05:29 +0800 Subject: [PATCH 2/3] make bazel Signed-off-by: AilinKid <314806019@qq.com> --- pkg/util/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/util/BUILD.bazel b/pkg/util/BUILD.bazel index 8b05a7d43f46b..e0155bb00b11a 100644 --- a/pkg/util/BUILD.bazel +++ b/pkg/util/BUILD.bazel @@ -62,6 +62,7 @@ go_test( "main_test.go", "misc_test.go", "prefix_helper_test.go", + "processinfo_test.go", "security_test.go", "urls_test.go", "util_test.go", From 8fb2e23e4e59e59be0368bc21cc255854b0e5d3a Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Wed, 24 Jan 2024 17:19:14 +0800 Subject: [PATCH 3/3] . Signed-off-by: AilinKid <314806019@qq.com> --- pkg/session/session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/session/session.go b/pkg/session/session.go index 818a71f191f45..12e33c356702a 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -1563,7 +1563,7 @@ func (s *session) UpdateProcessInfo() { // Update the current transaction start timestamp. shallowCP.CurTxnStartTS = s.sessionVars.TxnCtx.StartTS shallowCP.CurTxnCreateTime = s.sessionVars.TxnCtx.CreateTime - s.processInfo.Store(&shallowCP) + s.processInfo.Store(shallowCP) } func (s *session) getOomAlarmVariablesInfo() util.OOMAlarmVariablesInfo {