From ed3802d7e484059a4e72090096e5819d62593756 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 25 Nov 2024 11:43:17 +0800 Subject: [PATCH 1/7] statistics: add more tests for init stats Signed-off-by: Weizhen Wang --- pkg/statistics/handle/bootstrap.go | 18 +++--- .../handle/handletest/initstats/BUILD.bazel | 20 ++++++ .../handletest/initstats/load_stats_test.go | 64 +++++++++++++++++++ .../handle/handletest/initstats/main_test.go | 34 ++++++++++ 4 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 pkg/statistics/handle/handletest/initstats/BUILD.bazel create mode 100644 pkg/statistics/handle/handletest/initstats/load_stats_test.go create mode 100644 pkg/statistics/handle/handletest/initstats/main_test.go diff --git a/pkg/statistics/handle/bootstrap.go b/pkg/statistics/handle/bootstrap.go index 8504e18f3e53a..48d15215b75ae 100644 --- a/pkg/statistics/handle/bootstrap.go +++ b/pkg/statistics/handle/bootstrap.go @@ -363,7 +363,7 @@ func (h *Handle) initStatsHistogramsConcurrency(is infoschema.InfoSchema, cache } func (*Handle) initStatsTopN4Chunk(cache statstypes.StatsCache, iter *chunk.Iterator4Chunk, totalMemory uint64) { - if isFullCache(cache, totalMemory) { + if IsFullCacheFunc(cache, totalMemory) { return } affectedIndexes := make(map[*statistics.Index]struct{}) @@ -458,20 +458,20 @@ func (h *Handle) initStatsTopNByPaging(cache statstypes.StatsCache, task initsta } func (h *Handle) initStatsTopNConcurrency(cache statstypes.StatsCache, totalMemory uint64) error { - if isFullCache(cache, totalMemory) { + if IsFullCacheFunc(cache, totalMemory) { return nil } var maxTid = maxTidRecord.tid.Load() tid := int64(0) ls := initstats.NewRangeWorker("TopN", func(task initstats.Task) error { - if isFullCache(cache, totalMemory) { + if IsFullCacheFunc(cache, totalMemory) { return nil } return h.initStatsTopNByPaging(cache, task, totalMemory) }, uint64(maxTid), uint64(initStatsStep), initStatsPercentageInterval) ls.LoadStats() for tid <= maxTid { - if isFullCache(cache, totalMemory) { + if IsFullCacheFunc(cache, totalMemory) { break } ls.SendTask(initstats.Task{ @@ -620,7 +620,7 @@ func (*Handle) initStatsBuckets4Chunk(cache statstypes.StatsCache, iter *chunk.I } func (h *Handle) initStatsBuckets(cache statstypes.StatsCache, totalMemory uint64) error { - if isFullCache(cache, totalMemory) { + if IsFullCacheFunc(cache, totalMemory) { return nil } if config.GetGlobalConfig().Performance.ConcurrentlyInitStats { @@ -691,13 +691,13 @@ func (h *Handle) initStatsBucketsByPaging(cache statstypes.StatsCache, task init } func (h *Handle) initStatsBucketsConcurrency(cache statstypes.StatsCache, totalMemory uint64) error { - if isFullCache(cache, totalMemory) { + if IsFullCacheFunc(cache, totalMemory) { return nil } var maxTid = maxTidRecord.tid.Load() tid := int64(0) ls := initstats.NewRangeWorker("bucket", func(task initstats.Task) error { - if isFullCache(cache, totalMemory) { + if IsFullCacheFunc(cache, totalMemory) { return nil } return h.initStatsBucketsByPaging(cache, task) @@ -709,7 +709,7 @@ func (h *Handle) initStatsBucketsConcurrency(cache statstypes.StatsCache, totalM EndTid: tid + initStatsStep, }) tid += initStatsStep - if isFullCache(cache, totalMemory) { + if IsFullCacheFunc(cache, totalMemory) { break } } @@ -809,6 +809,8 @@ func (h *Handle) InitStats(ctx context.Context, is infoschema.InfoSchema) (err e return nil } +var IsFullCacheFunc func(cache statstypes.StatsCache, total uint64) bool = isFullCache + func isFullCache(cache statstypes.StatsCache, total uint64) bool { memQuota := variable.StatsCacheMemQuota.Load() return (uint64(cache.MemConsumed()) >= total/4) || (cache.MemConsumed() >= memQuota && memQuota != 0) diff --git a/pkg/statistics/handle/handletest/initstats/BUILD.bazel b/pkg/statistics/handle/handletest/initstats/BUILD.bazel new file mode 100644 index 0000000000000..1c48b9739b799 --- /dev/null +++ b/pkg/statistics/handle/handletest/initstats/BUILD.bazel @@ -0,0 +1,20 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "initstats_test", + timeout = "short", + srcs = [ + "load_stats_test.go", + "main_test.go", + ], + flaky = True, + deps = [ + "//pkg/config", + "//pkg/statistics/handle", + "//pkg/statistics/handle/types", + "//pkg/testkit", + "//pkg/testkit/testsetup", + "@com_github_stretchr_testify//require", + "@org_uber_go_goleak//:goleak", + ], +) diff --git a/pkg/statistics/handle/handletest/initstats/load_stats_test.go b/pkg/statistics/handle/handletest/initstats/load_stats_test.go new file mode 100644 index 0000000000000..f7b63978016e7 --- /dev/null +++ b/pkg/statistics/handle/handletest/initstats/load_stats_test.go @@ -0,0 +1,64 @@ +// 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 initstats + +import ( + "context" + "fmt" + "testing" + + "github.com/pingcap/tidb/pkg/config" + "github.com/pingcap/tidb/pkg/statistics/handle" + statstypes "github.com/pingcap/tidb/pkg/statistics/handle/types" + "github.com/pingcap/tidb/pkg/testkit" + "github.com/stretchr/testify/require" +) + +func TestConcurrentlyInitStatsWithMemoryLimit(t *testing.T) { + restore := config.RestoreFunc() + defer restore() + config.UpdateGlobal(func(conf *config.Config) { + conf.Performance.LiteInitStats = false + conf.Performance.ConcurrentlyInitStats = true + }) + handle.IsFullCacheFunc = func(cache statstypes.StatsCache, total uint64) bool { + return true + } + store, dom := testkit.CreateMockStoreAndDomain(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table t1 (a int, b int, c int, primary key(a), key idx(b))") + tk.MustExec("insert into t1 values (1,1,1),(2,2,2),(3,3,3),(4,4,4),(5,5,5),(6,7,8)") + tk.MustExec("analyze table t1") + for i := 2; i < 10; i++ { + tk.MustExec(fmt.Sprintf("create table t%v (a int, b int, c int, primary key(a), key idx(b))", i)) + tk.MustExec(fmt.Sprintf("insert into t%v select * from t1", i)) + tk.MustExec(fmt.Sprintf("analyze table t%v", i)) + } + h := dom.StatsHandle() + is := dom.InfoSchema() + h.Clear() + require.Equal(t, h.MemConsumed(), int64(0)) + require.NoError(t, h.InitStats(context.Background(), is)) + for i := 1; i < 10; i++ { + tk.MustQuery(fmt.Sprintf("explain select * from t%v where a = 1", i)).CheckNotContain("pseudo") + } + for i := 1; i < 10; i++ { + tk.MustQuery(fmt.Sprintf("explain select * from t%v where b = 1", i)).CheckNotContain("pseudo") + } + for i := 1; i < 10; i++ { + tk.MustQuery(fmt.Sprintf("explain select * from t%v where c = 1", i)).CheckNotContain("pseudo") + } +} diff --git a/pkg/statistics/handle/handletest/initstats/main_test.go b/pkg/statistics/handle/handletest/initstats/main_test.go new file mode 100644 index 0000000000000..13d11c7eaef3f --- /dev/null +++ b/pkg/statistics/handle/handletest/initstats/main_test.go @@ -0,0 +1,34 @@ +// 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 initstats + +import ( + "testing" + + "github.com/pingcap/tidb/pkg/testkit/testsetup" + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + opts := []goleak.Option{ + goleak.IgnoreTopFunction("github.com/golang/glog.(*fileSink).flushDaemon"), + goleak.IgnoreTopFunction("github.com/bazelbuild/rules_go/go/tools/bzltestutil.RegisterTimeoutHandler.func1"), + goleak.IgnoreTopFunction("github.com/lestrrat-go/httprc.runFetchWorker"), + goleak.IgnoreTopFunction("go.etcd.io/etcd/client/pkg/v3/logutil.(*MergeLogger).outputLoop"), + goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"), + } + testsetup.SetupForCommonTest() + goleak.VerifyTestMain(m, opts...) +} From 83c339b7eab4228d89fd11517a23e6002d5377a7 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 25 Nov 2024 14:17:28 +0800 Subject: [PATCH 2/7] update --- pkg/statistics/handle/bootstrap.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/statistics/handle/bootstrap.go b/pkg/statistics/handle/bootstrap.go index 48d15215b75ae..4913153acc475 100644 --- a/pkg/statistics/handle/bootstrap.go +++ b/pkg/statistics/handle/bootstrap.go @@ -809,6 +809,7 @@ func (h *Handle) InitStats(ctx context.Context, is infoschema.InfoSchema) (err e return nil } +// IsFullCacheFunc is whether the cache is full or not. but we can only change it when to test var IsFullCacheFunc func(cache statstypes.StatsCache, total uint64) bool = isFullCache func isFullCache(cache statstypes.StatsCache, total uint64) bool { From e65f383f5e4db74391abff9fec0b53de7bb41770 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Tue, 26 Nov 2024 11:47:09 +0800 Subject: [PATCH 3/7] update Signed-off-by: Weizhen Wang --- .../handle/handletest/initstats/load_stats_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/statistics/handle/handletest/initstats/load_stats_test.go b/pkg/statistics/handle/handletest/initstats/load_stats_test.go index f7b63978016e7..6dd7fded339dd 100644 --- a/pkg/statistics/handle/handletest/initstats/load_stats_test.go +++ b/pkg/statistics/handle/handletest/initstats/load_stats_test.go @@ -52,6 +52,11 @@ func TestConcurrentlyInitStatsWithMemoryLimit(t *testing.T) { h.Clear() require.Equal(t, h.MemConsumed(), int64(0)) require.NoError(t, h.InitStats(context.Background(), is)) + for _, val := range h.StatsCache.Values() { + for _, col := range val.HistColl.GetColSlice() { + require.False(t, col.IsFullLoad()) + } + } for i := 1; i < 10; i++ { tk.MustQuery(fmt.Sprintf("explain select * from t%v where a = 1", i)).CheckNotContain("pseudo") } @@ -61,4 +66,9 @@ func TestConcurrentlyInitStatsWithMemoryLimit(t *testing.T) { for i := 1; i < 10; i++ { tk.MustQuery(fmt.Sprintf("explain select * from t%v where c = 1", i)).CheckNotContain("pseudo") } + for _, val := range h.StatsCache.Values() { + for _, col := range val.HistColl.GetColSlice() { + require.False(t, col.IsAllEvicted()) + } + } } From 4985d52aa0fe10bae363aeccb0ffa6b546053393 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 27 Nov 2024 13:37:34 +0800 Subject: [PATCH 4/7] update Signed-off-by: Weizhen Wang --- .../handle/handletest/initstats/BUILD.bazel | 1 + .../handletest/initstats/load_stats_test.go | 44 +++++++++++++++---- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/pkg/statistics/handle/handletest/initstats/BUILD.bazel b/pkg/statistics/handle/handletest/initstats/BUILD.bazel index 1c48b9739b799..c5c3b99b15a86 100644 --- a/pkg/statistics/handle/handletest/initstats/BUILD.bazel +++ b/pkg/statistics/handle/handletest/initstats/BUILD.bazel @@ -10,6 +10,7 @@ go_test( flaky = True, deps = [ "//pkg/config", + "//pkg/parser/model", "//pkg/statistics/handle", "//pkg/statistics/handle/types", "//pkg/testkit", diff --git a/pkg/statistics/handle/handletest/initstats/load_stats_test.go b/pkg/statistics/handle/handletest/initstats/load_stats_test.go index 6dd7fded339dd..89220e4808c30 100644 --- a/pkg/statistics/handle/handletest/initstats/load_stats_test.go +++ b/pkg/statistics/handle/handletest/initstats/load_stats_test.go @@ -20,8 +20,9 @@ import ( "testing" "github.com/pingcap/tidb/pkg/config" + "github.com/pingcap/tidb/pkg/parser/model" "github.com/pingcap/tidb/pkg/statistics/handle" - statstypes "github.com/pingcap/tidb/pkg/statistics/handle/types" + "github.com/pingcap/tidb/pkg/statistics/handle/types" "github.com/pingcap/tidb/pkg/testkit" "github.com/stretchr/testify/require" ) @@ -33,17 +34,34 @@ func TestConcurrentlyInitStatsWithMemoryLimit(t *testing.T) { conf.Performance.LiteInitStats = false conf.Performance.ConcurrentlyInitStats = true }) - handle.IsFullCacheFunc = func(cache statstypes.StatsCache, total uint64) bool { + handle.IsFullCacheFunc = func(cache types.StatsCache, total uint64) bool { return true } + testConcurrentlyInitStats(t) +} + +func TestConcurrentlyInitStatsWithoutMemoryLimit(t *testing.T) { + restore := config.RestoreFunc() + defer restore() + config.UpdateGlobal(func(conf *config.Config) { + conf.Performance.LiteInitStats = false + conf.Performance.ConcurrentlyInitStats = true + }) + handle.IsFullCacheFunc = func(cache types.StatsCache, total uint64) bool { + return false + } + testConcurrentlyInitStats(t) +} + +func testConcurrentlyInitStats(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") - tk.MustExec("create table t1 (a int, b int, c int, primary key(a), key idx(b))") + tk.MustExec("create table t1 (a int, b int, c int, primary key(c))") tk.MustExec("insert into t1 values (1,1,1),(2,2,2),(3,3,3),(4,4,4),(5,5,5),(6,7,8)") tk.MustExec("analyze table t1") for i := 2; i < 10; i++ { - tk.MustExec(fmt.Sprintf("create table t%v (a int, b int, c int, primary key(a), key idx(b))", i)) + tk.MustExec(fmt.Sprintf("create table t%v (a int, b int, c int, primary key(c))", i)) tk.MustExec(fmt.Sprintf("insert into t%v select * from t1", i)) tk.MustExec(fmt.Sprintf("analyze table t%v", i)) } @@ -52,8 +70,13 @@ func TestConcurrentlyInitStatsWithMemoryLimit(t *testing.T) { h.Clear() require.Equal(t, h.MemConsumed(), int64(0)) require.NoError(t, h.InitStats(context.Background(), is)) - for _, val := range h.StatsCache.Values() { - for _, col := range val.HistColl.GetColSlice() { + for i := 1; i < 10; i++ { + tbl, err := is.TableByName(context.Background(), model.NewCIStr("test"), model.NewCIStr(fmt.Sprintf("t%v", i))) + require.NoError(t, err) + stats, ok := h.StatsCache.Get(tbl.Meta().ID) + require.True(t, ok) + for _, col := range stats.GetColSlice() { + require.True(t, col.IsAllEvicted()) require.False(t, col.IsFullLoad()) } } @@ -66,8 +89,13 @@ func TestConcurrentlyInitStatsWithMemoryLimit(t *testing.T) { for i := 1; i < 10; i++ { tk.MustQuery(fmt.Sprintf("explain select * from t%v where c = 1", i)).CheckNotContain("pseudo") } - for _, val := range h.StatsCache.Values() { - for _, col := range val.HistColl.GetColSlice() { + for i := 1; i < 10; i++ { + tbl, err := is.TableByName(context.Background(), model.NewCIStr("test"), model.NewCIStr(fmt.Sprintf("t%v", i))) + require.NoError(t, err) + stats, ok := h.StatsCache.Get(tbl.Meta().ID) + require.True(t, ok) + for _, col := range stats.GetColSlice() { + require.True(t, col.IsFullLoad()) require.False(t, col.IsAllEvicted()) } } From 17ade7df13cb76760a28ddadb83e1831056a26e3 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 27 Nov 2024 14:19:35 +0800 Subject: [PATCH 5/7] update Signed-off-by: Weizhen Wang --- pkg/statistics/handle/handletest/initstats/BUILD.bazel | 1 + tools/tazel/main.go | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/statistics/handle/handletest/initstats/BUILD.bazel b/pkg/statistics/handle/handletest/initstats/BUILD.bazel index c5c3b99b15a86..4687e03386821 100644 --- a/pkg/statistics/handle/handletest/initstats/BUILD.bazel +++ b/pkg/statistics/handle/handletest/initstats/BUILD.bazel @@ -8,6 +8,7 @@ go_test( "main_test.go", ], flaky = True, + shard_count = 2, deps = [ "//pkg/config", "//pkg/parser/model", diff --git a/tools/tazel/main.go b/tools/tazel/main.go index 7a154a0ac6ef2..f021401e57c18 100644 --- a/tools/tazel/main.go +++ b/tools/tazel/main.go @@ -63,8 +63,6 @@ func main() { if cnt > 2 { gotest[0].SetAttr("shard_count", &build.LiteralExpr{Token: strconv.FormatUint(uint64(min(cnt, maxShardCount)), 10)}) - } else { - gotest[0].DelAttr("shard_count") } } } From 5469811360a19aed2d95de23991b3dddd96963dc Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Thu, 28 Nov 2024 17:14:26 +0800 Subject: [PATCH 6/7] update Signed-off-by: Weizhen Wang --- pkg/statistics/handle/handletest/initstats/load_stats_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/statistics/handle/handletest/initstats/load_stats_test.go b/pkg/statistics/handle/handletest/initstats/load_stats_test.go index 89220e4808c30..8fcaaa769f602 100644 --- a/pkg/statistics/handle/handletest/initstats/load_stats_test.go +++ b/pkg/statistics/handle/handletest/initstats/load_stats_test.go @@ -57,13 +57,14 @@ func testConcurrentlyInitStats(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") + tk.MustExec("set global tidb_analyze_column_options='ALL'") tk.MustExec("create table t1 (a int, b int, c int, primary key(c))") tk.MustExec("insert into t1 values (1,1,1),(2,2,2),(3,3,3),(4,4,4),(5,5,5),(6,7,8)") tk.MustExec("analyze table t1") for i := 2; i < 10; i++ { tk.MustExec(fmt.Sprintf("create table t%v (a int, b int, c int, primary key(c))", i)) tk.MustExec(fmt.Sprintf("insert into t%v select * from t1", i)) - tk.MustExec(fmt.Sprintf("analyze table t%v", i)) + tk.MustExec(fmt.Sprintf("analyze table t%v all columns", i)) } h := dom.StatsHandle() is := dom.InfoSchema() From d2b5910fa863b084d29fe98277bcbbaf3dd351f2 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Thu, 28 Nov 2024 17:22:35 +0800 Subject: [PATCH 7/7] update Signed-off-by: Weizhen Wang --- pkg/statistics/handle/handletest/initstats/BUILD.bazel | 1 - tools/tazel/main.go | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/statistics/handle/handletest/initstats/BUILD.bazel b/pkg/statistics/handle/handletest/initstats/BUILD.bazel index 4687e03386821..c5c3b99b15a86 100644 --- a/pkg/statistics/handle/handletest/initstats/BUILD.bazel +++ b/pkg/statistics/handle/handletest/initstats/BUILD.bazel @@ -8,7 +8,6 @@ go_test( "main_test.go", ], flaky = True, - shard_count = 2, deps = [ "//pkg/config", "//pkg/parser/model", diff --git a/tools/tazel/main.go b/tools/tazel/main.go index f021401e57c18..7a154a0ac6ef2 100644 --- a/tools/tazel/main.go +++ b/tools/tazel/main.go @@ -63,6 +63,8 @@ func main() { if cnt > 2 { gotest[0].SetAttr("shard_count", &build.LiteralExpr{Token: strconv.FormatUint(uint64(min(cnt, maxShardCount)), 10)}) + } else { + gotest[0].DelAttr("shard_count") } } }