Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upsql: deflake TestSchemaChangeReverseMutations #31515
Conversation
eriktrinh
requested a review
from
vivekmenezes
Oct 16, 2018
eriktrinh
requested a review
from cockroachdb/sql-async-prs
as a
code owner
Oct 16, 2018
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
vivekmenezes
requested changes
Oct 16, 2018
Reviewable status:
complete! 0 of 0 LGTMs obtained
pkg/sql/schema_changer_test.go, line 1779 at r1 (raw file):
if _, err := addDefaultZoneConfig(sqlDB, tableDesc.ID); err != nil { t.Fatal(err) }
Any particular reason to need to reset it?
eriktrinh
reviewed
Oct 16, 2018
Reviewable status:
complete! 0 of 0 LGTMs obtained
pkg/sql/schema_changer_test.go, line 1779 at r1 (raw file):
Previously, vivekmenezes wrote…
Any particular reason to need to reset it?
The error is the same as #31177 and probably happens because the GC TTL is set to 0 so there is a race between the gc running and when the subsequent reads of this test occur. Resetting the GC TTL should prevent that.
eriktrinh
reviewed
Oct 16, 2018
Reviewable status:
complete! 0 of 0 LGTMs obtained
pkg/sql/schema_changer_test.go, line 1779 at r1 (raw file):
Previously, eriktrinh (Erik Trinh) wrote…
The error is the same as #31177 and probably happens because the GC TTL is set to 0 so there is a race between the gc running and when the subsequent reads of this test occur. Resetting the GC TTL should prevent that.
Restructured as discussed.
vivekmenezes
approved these changes
Oct 16, 2018
Reviewable status:
complete! 0 of 0 LGTMs obtained
pkg/sql/schema_changer_test.go, line 1771 at r2 (raw file):
// Wait until all the mutations have been processed. var rows *gosql.Rows
Do we need this to be defined?
eriktrinh
reviewed
Oct 16, 2018
Reviewable status:
complete! 0 of 0 LGTMs obtained
pkg/sql/schema_changer_test.go, line 1771 at r2 (raw file):
Previously, vivekmenezes wrote…
Do we need this to be defined?
Done.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
bors r+ |
bot
pushed a commit
that referenced
this pull request
Oct 17, 2018
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
craig
bot
commented
Oct 17, 2018
Build succeeded |
eriktrinh commentedOct 16, 2018
Fixes #31462.
Release note: None