catalog/lease: deflake TestLeaseAtLatestVersion #139497
Conversation
|
This change is |
fqazi
force-pushed
the
TestLeaseAtLatestVersionFlake
branch
from
65ed92a to
9b293a5
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @fqazi)
pkg/sql/catalog/lease/lease_test.go line 1161 at r1 (raw file):
s := srv.ApplicationLayer() // This test relies on the initial version not being acquired. if _, err := sqlDB.Exec("SET CLUSTER SETTING sql.catalog.descriptor_wait_for_initial_version.enabled=false"); err != nil {
out of curiosity, how much work would it be to make this test work with this setting enabled?
fqazi
force-pushed
the
TestLeaseAtLatestVersionFlake
branch
2 times, most recently
from
8b9c426 to
230ce05
Compare
Currently, TestLeaseAtLatestVersion can flake when the initial version of a descriptor is acquired, which this test is not designed to handle. To address this, this patch will intentionally use a different database for the timestamp table, so that publishing a new version or a slow range feed does not acquire the kv table in the same schema. Fixes: cockroachdb#139386 Release note: None
fqazi
force-pushed
the
TestLeaseAtLatestVersionFlake
branch
from
230ce05 to
29d1a6b
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/catalog/lease/lease_test.go line 1161 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
out of curiosity, how much work would it be to make this test work with this setting enabled?
Done.
Fair point, and a simpler workaround is to just put timestamp in its own schema. The initial version is fetched because the schema gets leased by the insert, breaking the assumptions of the test
There was a problem hiding this comment.
nice fix!
Reviewable status:
|
@rafiss TFTR! bors r+ |
|
bors r+ |
Currently, TestLeaseAtLatestVersion can flake when the initial version
of a descriptor is acquired, which this test is not designed to handle.
To address this, this patch will intentionally use a different database
for the timestamp table, so that publishing a new version or a slow
range feed does not acquire the kv table in the same schema.
Fixes: #139386
Release note: None