Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

statistics: fix wrong NDV in the global stats when to disable async-merge-global-stats #53762

Merged
merged 10 commits into from
Jun 4, 2024
2 changes: 1 addition & 1 deletion pkg/statistics/handle/globalstats/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ go_test(
],
embed = [":globalstats"],
flaky = True,
shard_count = 26,
shard_count = 28,
deps = [
"//pkg/domain",
"//pkg/parser/model",
Expand Down
9 changes: 7 additions & 2 deletions pkg/statistics/handle/globalstats/global_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,15 @@ func blockingMergePartitionStats2GlobalStats(
}
// FMSketch use many memory, so we first deal with it and then destroy it.
// Merge FMSketch.
// NOTE: allFms maybe contain empty.
globalStats.Fms[i] = allFms[i][0]
for j := 1; j < len(allFms[i]); j++ {
globalStats.Fms[i].MergeFMSketch(allFms[i][j])
allFms[i][j].DestroyAndPutToPool()
if globalStats.Fms[i] == nil {
globalStats.Fms[i] = allFms[i][j]
} else {
globalStats.Fms[i].MergeFMSketch(allFms[i][j])
allFms[i][j].DestroyAndPutToPool()
}
}

// Update the global NDV.
Expand Down
72 changes: 72 additions & 0 deletions pkg/statistics/handle/globalstats/global_stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -959,3 +959,75 @@ func TestGlobalStatsAndSQLBindingWithConcurrency(t *testing.T) {
tk.MustExec("set global tidb_merge_partition_stats_concurrency=2")
testGlobalStatsAndSQLBinding(tk)
}

func TestBlockCheckFMSketch(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that even if I revert your change I can still pass the tests. So maybe the test case doesn't cover the change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should disable the tidb_enable_async_merge_global_stats.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have. I've run the TestBlockCheckFMSketch.

  1. I revert your change.
  2. I ran the TestBlockCheckFMSketch, and it still passed.

Copy link
Member

@Rustin170506 Rustin170506 Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this problem, only it is reproduced in the real tikv.

hawkingrei marked this conversation as resolved.
Show resolved Hide resolved
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("set @@tidb_enable_async_merge_global_stats=OFF;")
defer func() {
tk.MustExec("set @@tidb_enable_async_merge_global_stats=ON;")
}()
checkFMSketch(tk)
}

func TestAsyncCheckFMSketch(t *testing.T) {
hawkingrei marked this conversation as resolved.
Show resolved Hide resolved
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("set @@tidb_enable_async_merge_global_stats=ON;")
checkFMSketch(tk)
}

func checkFMSketch(tk *testkit.TestKit) {
tk.MustExec(`CREATE TABLE employees (id INT NOT NULL AUTO_INCREMENT PRIMARY KEY,fname VARCHAR(25) NOT NULL,lname VARCHAR(25) NOT NULL,store_id INT NOT NULL,department_id INT NOT NULL
) PARTITION BY RANGE(id) (
PARTITION p0 VALUES LESS THAN (5),
PARTITION p1 VALUES LESS THAN (10),
PARTITION p2 VALUES LESS THAN (15),
PARTITION p3 VALUES LESS THAN MAXVALUE
);`)
tk.MustExec(`INSERT INTO employees(FNAME,LNAME,STORE_ID,DEPARTMENT_ID) VALUES
('Bob', 'Taylor', 3, 2), ('Frank', 'Williams', 1, 2),
('Ellen', 'Johnson', 3, 4), ('Jim', 'Smith', 2, 4),
('Mary', 'Jones', 1, 1), ('Linda', 'Black', 2, 3),
('Ed', 'Jones', 2, 1), ('June', 'Wilson', 3, 1),
('Andy', 'Smith', 1, 3), ('Lou', 'Waters', 2, 4),
('Jill', 'Stone', 1, 4), ('Roger', 'White', 3, 2),
('Howard', 'Andrews', 1, 2), ('Fred', 'Goldberg', 3, 3),
('Barbara', 'Brown', 2, 3), ('Alice', 'Rogers', 2, 2),
('Mark', 'Morgan', 3, 3), ('Karen', 'Cole', 3, 2);`)
tk.MustExec("ANALYZE TABLE employees;")
tk.MustExec("select * from employees;")
tk.MustExec("alter table employees truncate partition p0 ; ")
hawkingrei marked this conversation as resolved.
Show resolved Hide resolved
tk.MustExec("select * from employees;")
tk.MustExec("analyze table employees partition p3;")
tk.MustExec("select * from employees;")
tk.MustQuery("select distinct_count, null_count, tot_col_size from mysql.stats_histograms where is_index=0 order by table_id asc").Check(
testkit.Rows("14 0 14",
"14 0 76",
"13 0 92",
"3 0 14",
"4 0 14",
"4 0 4",
"4 0 20",
"4 0 30",
"3 0 4",
"2 0 4",
"5 0 5",
"5 0 24",
"4 0 31",
"3 0 5",
"2 0 5",
"5 0 5",
"5 0 27",
"5 0 36",
"3 0 5",
"3 0 5",
"4 0 4",
"4 0 25",
"4 0 25",
"2 0 4",
"2 0 4",
))
}