Skip to content

Commit

Permalink
util: use go-deadlock to find deadlock (#40288)
Browse files Browse the repository at this point in the history
close #40293
  • Loading branch information
hawkingrei authored Jan 5, 2023
1 parent 4a5a447 commit 5eea731
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 8 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ bazel_test: failpoint-enable bazel_ci_prepare

bazel_coverage_test: failpoint-enable bazel_ci_prepare
bazel $(BAZEL_GLOBAL_CONFIG) coverage $(BAZEL_CMD_CONFIG) \
--build_event_json_file=bazel_1.json --@io_bazel_rules_go//go/config:cover_format=go_cover \
--build_event_json_file=bazel_1.json --@io_bazel_rules_go//go/config:cover_format=go_cover --define gotags=deadlock \
-- //... -//cmd/... -//tests/graceshutdown/... \
-//tests/globalkilltest/... -//tests/readonlytest/... -//br/pkg/task:task_test -//tests/realtikvtest/...

Expand Down
1 change: 1 addition & 0 deletions domain/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ go_library(
"//util/replayer",
"//util/servermemorylimit",
"//util/sqlexec",
"//util/syncutil",
"@com_github_burntsushi_toml//:toml",
"@com_github_ngaut_pools//:pools",
"@com_github_pingcap_errors//:errors",
Expand Down
10 changes: 5 additions & 5 deletions domain/sysvar_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ package domain
import (
"context"
"fmt"
"sync"

"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/variable"
"github.com/pingcap/tidb/util/logutil"
"github.com/pingcap/tidb/util/sqlexec"
"github.com/pingcap/tidb/util/syncutil"
"go.uber.org/zap"
"golang.org/x/exp/maps"
)
Expand All @@ -36,10 +36,10 @@ import (

// sysVarCache represents the cache of system variables broken up into session and global scope.
type sysVarCache struct {
sync.RWMutex // protects global and session maps
global map[string]string
session map[string]string
rebuildLock sync.Mutex // protects concurrent rebuild
syncutil.RWMutex // protects global and session maps
global map[string]string
session map[string]string
rebuildLock syncutil.Mutex // protects concurrent rebuild
}

func (do *Domain) rebuildSysVarCacheIfNeeded() (err error) {
Expand Down
1 change: 1 addition & 0 deletions executor/autoidtest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ go_test(
],
flaky = True,
race = "on",
shard_count = 5,
deps = [
"//autoid_service",
"//config",
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ require (
github.com/prometheus/client_model v0.3.0
github.com/prometheus/common v0.39.0
github.com/prometheus/prometheus v0.0.0-20190525122359-d20e84d0fb64
github.com/sasha-s/go-deadlock v0.0.0-20161201235124-341000892f3d
github.com/shirou/gopsutil/v3 v3.22.9
github.com/shurcooL/httpgzip v0.0.0-20190720172056-320755c1c1b0
github.com/soheilhy/cmux v0.1.5
Expand Down Expand Up @@ -203,6 +204,7 @@ require (
github.com/ngaut/sync2 v0.0.0-20141008032647-7a24ed77b2ef // indirect
github.com/oklog/ulid v1.3.1 // indirect
github.com/olekukonko/tablewriter v0.0.5 // indirect
github.com/petermattis/goid v0.0.0-20170504144140-0ded85884ba5 // indirect
github.com/pierrec/lz4 v2.6.1+incompatible // indirect
github.com/pingcap/check v0.0.0-20200212061837-5e12011dc712 // indirect
github.com/pingcap/goleveldb v0.0.0-20191226122134-f82aafb29989 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,7 @@ github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FI
github.com/pborman/getopt v0.0.0-20180729010549-6fdd0a2c7117/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o=
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
github.com/peterbourgon/g2s v0.0.0-20170223122336-d4e7ad98afea/go.mod h1:1VcHEd3ro4QMoHfiNl/j7Jkln9+KQuorp0PItHMJYNg=
github.com/petermattis/goid v0.0.0-20170504144140-0ded85884ba5 h1:rUMC+oZ89Om6l9wvUNjzI0ZrKrSnXzV+opsgAohYUNc=
github.com/petermattis/goid v0.0.0-20170504144140-0ded85884ba5/go.mod h1:jvVRKCrJTQWu0XVbaOlby/2lO20uSCHEMzzplHXte1o=
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2 h1:JhzVVoYvbOACxoUmOs6V/G4D5nPVUW73rKvXxP4XUJc=
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2/go.mod h1:iIss55rKnNBTvrwdmkUpLnDpZoAHvWaiq5+iMmen4AE=
Expand Down Expand Up @@ -861,6 +862,7 @@ github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD
github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/samuel/go-zookeeper v0.0.0-20161028232340-1d7be4effb13/go.mod h1:gi+0XIa01GRL2eRQVjQkKGqKF3SF9vZR/HnPullcV2E=
github.com/sasha-s/go-deadlock v0.0.0-20161201235124-341000892f3d h1:yVBZEAirqhDYAc7xftf/swe8eHcg63jqfwdqN8KSoR8=
github.com/sasha-s/go-deadlock v0.0.0-20161201235124-341000892f3d/go.mod h1:StQn567HiB1fF2yJ44N9au7wOhrPS3iZqiDbRupzT10=
github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0=
github.com/sclevine/agouti v3.0.0+incompatible/go.mod h1:b4WX9W9L1sfQKXeJf1mUTLZKJ48R1S7H23Ji7oFO5Bw=
Expand Down
1 change: 1 addition & 0 deletions server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ go_test(
"//util/resourcegrouptag",
"//util/rowcodec",
"//util/sqlexec",
"//util/syncutil",
"//util/topsql",
"//util/topsql/collector",
"//util/topsql/collector/mock",
Expand Down
7 changes: 6 additions & 1 deletion server/tidb_library_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/docker/go-units"
"github.com/pingcap/tidb/session"
"github.com/pingcap/tidb/store/mockstore"
"github.com/pingcap/tidb/util/syncutil"
"github.com/stretchr/testify/require"
)

Expand All @@ -47,5 +48,9 @@ func TestMemoryLeak(t *testing.T) {
runtime.GC()
runtime.ReadMemStats(&memStat)
// before the fix, initAndCloseTiDB for 20 times will cost 900 MB memory, so we test for a quite loose upper bound.
require.Less(t, memStat.HeapInuse-oldHeapInUse, uint64(300*units.MiB))
if syncutil.EnableDeadlock {
require.Less(t, memStat.HeapInuse-oldHeapInUse, uint64(1100*units.MiB))
} else {
require.Less(t, memStat.HeapInuse-oldHeapInUse, uint64(300*units.MiB))
}
}
1 change: 1 addition & 0 deletions session/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ go_library(
"//util/sem",
"//util/sli",
"//util/sqlexec",
"//util/syncutil",
"//util/tableutil",
"//util/timeutil",
"//util/topsql",
Expand Down
3 changes: 2 additions & 1 deletion session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ import (
"github.com/pingcap/tidb/util/sem"
"github.com/pingcap/tidb/util/sli"
"github.com/pingcap/tidb/util/sqlexec"
"github.com/pingcap/tidb/util/syncutil"
"github.com/pingcap/tidb/util/tableutil"
"github.com/pingcap/tidb/util/timeutil"
"github.com/pingcap/tidb/util/topsql"
Expand Down Expand Up @@ -274,7 +275,7 @@ type session struct {
idxUsageCollector *handle.SessionIndexUsageCollector

functionUsageMu struct {
sync.RWMutex
syncutil.RWMutex
builtinFunctionUsage telemetry.BuiltinFunctionsUsage
}
// allowed when tikv disk full happened.
Expand Down
12 changes: 12 additions & 0 deletions util/syncutil/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "syncutil",
srcs = [
"mutex_deadlock.go",
"mutex_sync.go",
],
importpath = "github.com/pingcap/tidb/util/syncutil",
visibility = ["//visibility:public"],
deps = ["@com_github_sasha_s_go_deadlock//:go-deadlock"],
)
40 changes: 40 additions & 0 deletions util/syncutil/mutex_deadlock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2023 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.

//go:build deadlock

package syncutil

import (
"time"

deadlock "github.com/sasha-s/go-deadlock"
)

// EnableDeadlock is a flag to enable deadlock detection.
const EnableDeadlock = true

func init() {
deadlock.Opts.DeadlockTimeout = 20 * time.Second
}

// A Mutex is a mutual exclusion lock.
type Mutex struct {
deadlock.Mutex
}

// An RWMutex is a reader/writer mutual exclusion lock.
type RWMutex struct {
deadlock.RWMutex
}
32 changes: 32 additions & 0 deletions util/syncutil/mutex_sync.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2023 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.

//go:build !deadlock

package syncutil

import "sync"

// EnableDeadlock is a flag to enable deadlock detection.
const EnableDeadlock = false

// A Mutex is a mutual exclusion lock.
type Mutex struct {
sync.Mutex
}

// An RWMutex is a reader/writer mutual exclusion lock.
type RWMutex struct {
sync.RWMutex
}

0 comments on commit 5eea731

Please sign in to comment.