roachtest: don't reuse clusters that call dmsetup #108115
Conversation
|
This change is |
andrewbaptist
requested review from
herkolategan and
smg260
and removed request for
a team
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @andrewbaptist, @erikgrinaker, @herkolategan, and @smg260)
pkg/cmd/roachtest/tests/failover.go line 83 at r1 (raw file):
Benchmark: true, Timeout: 60 * time.Minute, Cluster: r.MakeClusterSpec(10, spec.CPU(2), spec.PreferLocalSSD(false), spec.ReuseNone()), // uses disk stalls
Nit: remove everything but the cluster size, since they are applied below; otherwise, it's redundant.
There was a problem hiding this comment.
Thanks!
mount/unmount
nit: unmount/mount works just fine, it's dmsetup that fails. The filesystem isn't in use, the device is.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
pkg/cmd/roachtest/tests/failover.go line 83 at r1 (raw file):
Previously, srosenberg (Stan Rosenberg) wrote…
Nit: remove everything but the cluster size, since they are applied below; otherwise, it's redundant.
This is a different test than the ones below that use clusterOpts. This one doesn't need conditional options.
pkg/cmd/roachtest/tests/failover.go line 133 at r1 (raw file):
// SSDs. See #97968. clusterOpts = append(clusterOpts, spec.PreferLocalSSD(false)) clusterOpts = append(clusterOpts, spec.ReuseNone())
nit: let's add a comment for why we set this.
Certain tests need to modify the blockdevice and they are prone to failures during setup that the device is still busy. Ideally we would figure out what is still holding onto the dish handle, but it is safer to simply not reuse clusters that perform this by adding `spec.ReuseNone()` Fixes: cockroachdb#107865 Epic: none Release note: None
andrewbaptist
force-pushed
the
2023-08-03-noreuse-disk
branch
from
ed7f423 to
704e6e9
Compare
There was a problem hiding this comment.
Changed
Reviewable status:
pkg/cmd/roachtest/tests/failover.go line 83 at r1 (raw file):
Previously, erikgrinaker (Erik Grinaker) wrote…
This is a different test than the ones below that use
clusterOpts. This one doesn't need conditional options.
Yep - as Erik mentioned, this test also needs it.
pkg/cmd/roachtest/tests/failover.go line 133 at r1 (raw file):
Previously, erikgrinaker (Erik Grinaker) wrote…
nit: let's add a comment for why we set this.
Added
andrewbaptist
changed the title
|
bors r=erikgrinaker, srosenberg |
|
Build failed (retrying...): |
108115: roachtest: don't reuse clusters that call dmsetup r=erikgrinaker,srosenberg a=andrewbaptist Certain tests need to modify the blockdevice and they are prone to failures during setup that the device is still busy. Ideally we would figure out what is still holding onto the dish handle, but it is safer to simply not reuse clusters that perform this by adding `spec.ReuseNone()` Fixes: #107865 Epic: none Release note: None 108165: sql: fix insight integration test for contention r=koorosh a=koorosh This change lowers the latency threshold for which statement is considered slow to make sure it detects contention. Previous value was set to 100ms which is default. New value is set to 30ms to be the same as overridden value in other tests. Release note: None Resolves: #108020 108192: ccl/sqlproxyccl: deflake TestConnectionMigration r=darinpp a=jaylim-crl Fixes #106885. This test flake seems extremely rare, and it's unclear why it occurred in the first place. The past 1000 runs (all of what TC has) have been successful. Regardless, this commit attempts at deflaking TestConnectionMigration. Given that some subtests transfer connections through `transferConnWithRetries`, it is possible that the transfer was retried, causing the metric to be incremented. Release note: None Epic: none Co-authored-by: Andrew Baptist <[email protected]> Co-authored-by: Andrii Vorobiov <[email protected]> Co-authored-by: Jay <[email protected]>
|
Build failed (retrying...): |
|
Build failed: |
|
bors retry |
|
Build succeeded: |
|
blathers backport 23.1 |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 704e6e9 to blathers/backport-release-23.1-108115: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1 failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Certain tests need to modify the blockdevice and they are prone to failures during setup that the device is still busy. Ideally we would figure out what is still holding onto the dish handle, but it is safer to simply not reuse clusters that perform this by adding
spec.ReuseNone()Fixes: #107865
Epic: none
Release note: None