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

ddl: fix unstable tests #26513

Merged
merged 10 commits into from
Aug 5, 2021
7 changes: 3 additions & 4 deletions ddl/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ func backgroundExec(s kv.Storage, sql string, done chan error) {

// TestAddPrimaryKeyRollback1 is used to test scenarios that will roll back when a duplicate primary key is encountered.
func (s *testDBSuite8) TestAddPrimaryKeyRollback1(c *C) {
c.Skip("unstable, skip it and fix it before 20210705")
hasNullValsInKey := false
idxName := "PRIMARY"
addIdxSQL := "alter table t1 add primary key c3_index (c3);"
Expand All @@ -312,7 +311,6 @@ func (s *testDBSuite8) TestAddPrimaryKeyRollback2(c *C) {
}

func (s *testDBSuite2) TestAddUniqueIndexRollback(c *C) {
c.Skip("unstable, skip it and fix it before 20210702")
hasNullValsInKey := false
idxName := "c3_index"
addIdxSQL := "create unique index c3_index on t1 (c3)"
Expand Down Expand Up @@ -516,8 +514,9 @@ LOOP:
// delete some rows, and add some data
for i := count; i < count+step; i++ {
n := rand.Intn(count)
// Don't delete this row, otherwise error message would change.
if n == defaultBatchSize*2-10 {
// (2048, 2038, 2038) and (2038, 2038, 2038)
Copy link
Member

Choose a reason for hiding this comment

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

I don't get the comment here. Could you add an explanation with more details?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are the two duplicate rows. So we shouldn't delete either of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

rand.Intn(2048) won't return 2048, according to the doc: https://pkg.go.dev/math/rand#Rand.Intn

BTW, it's a little hard to understand. Why not just remove data in the range of [0, 2037]?

Copy link
Member Author

Choose a reason for hiding this comment

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

count += step, so it may > 2048.
This test wants to randomly delete some rows. So it doesn't remove data in the range of [0, 2037] directly.
It's not easy to understand, that why I missed this check at the first fix.

// Don't delete rows where c1 is 2048 or 2038, otherwise, the entry value in duplicated error message would change.
if n == defaultBatchSize*2-10 || n == defaultBatchSize*2 {
continue
}
tk.MustExec("delete from t1 where c1 = ?", n)
Expand Down