Skip to content

roachtest: use local SSDs for disk-stall failover tests#99963

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nicktrav:nickt.failover-disk-stall
Mar 30, 2023
Merged

roachtest: use local SSDs for disk-stall failover tests#99963
craig[bot] merged 1 commit intocockroachdb:masterfrom
nicktrav:nickt.failover-disk-stall

Conversation

@nicktrav
Copy link
Copy Markdown
Collaborator

The disk-stalled roachtests were updated in #99747 to use local SSDs. This change broke the failover/*/disk-stall tests, which look for /dev/sdb on GCE (the used for GCE Persistent Disks), but the tests still create clusters with local SSDs (the roachtest default).

Fix #99902.
Fix #99926.
Fix #99930.

Touches #97968.

Release note: None.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@nicktrav nicktrav added the backport-23.1.x PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only label Mar 29, 2023
@nicktrav
Copy link
Copy Markdown
Collaborator Author

I took this for a spin. Each of the three variants passed:

@nicktrav nicktrav requested review from a team, andrewbaptist and sumeerbhola March 29, 2023 20:29
@nicktrav nicktrav marked this pull request as ready for review March 29, 2023 20:29
@nicktrav nicktrav requested a review from a team as a code owner March 29, 2023 20:29
@nicktrav nicktrav requested review from herkolategan and smg260 and removed request for a team March 29, 2023 20:29
Copy link
Copy Markdown

@andrewbaptist andrewbaptist left a comment

Choose a reason for hiding this comment

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

:lgtm:

Copy link
Copy Markdown
Collaborator

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @herkolategan, @nicktrav, and @smg260)


-- commits line 4 at r1:
nit: to not use?


-- commits line 6 at r1:
nit: something missing/odd in "the used for ..."


pkg/cmd/roachtest/tests/disk_stall.go line 51 at r1 (raw file):

		s := r.MakeClusterSpec(4, spec.ReuseNone())
		// Use local SSDs in an attempt to work around flakes encountered when using
		// SSDs. See #97968.

Confused here too. #99747 says switch to PD.


pkg/cmd/roachtest/tests/failover.go line 52 at r1 (raw file):

			if failureMode == failureModeDiskStall {
				// Use local SSDs in an attempt to work around flakes encountered when
				// using SSDs. See #97968.

ditto

@nicktrav nicktrav force-pushed the nickt.failover-disk-stall branch 2 times, most recently from 08b5ef6 to fce87cd Compare March 29, 2023 22:38
Copy link
Copy Markdown
Collaborator Author

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @smg260, and @sumeerbhola)


-- commits line 4 at r1:

Previously, sumeerbhola wrote…

nit: to not use?

Woops. Yes, confusing myself. Fixed.


-- commits line 6 at r1:

Previously, sumeerbhola wrote…

nit: something missing/odd in "the used for ..."

Fixed.


pkg/cmd/roachtest/tests/disk_stall.go line 51 at r1 (raw file):

Previously, sumeerbhola wrote…

Confused here too. #99747 says switch to PD.

Done.


pkg/cmd/roachtest/tests/failover.go line 52 at r1 (raw file):

Previously, sumeerbhola wrote…

ditto

Done.

The disk-stalled roachtests were updated in cockroachdb#99747 to use PDs in favor
of local SSDs. This change broke the `failover/*/disk-stall` tests,
which look for `/dev/sdb` on GCE (the device name used for GCE
Persistent Disks), but the tests still create clusters with local SSDs
(the roachtest default).

Fix cockroachdb#99902.
Fix cockroachdb#99926.
Fix cockroachdb#99930.

Touches cockroachdb#97968.

Release note: None.
@nicktrav nicktrav force-pushed the nickt.failover-disk-stall branch from fce87cd to 545d49a Compare March 29, 2023 23:45
@nicktrav
Copy link
Copy Markdown
Collaborator Author

Rebasing around some CI flakes.

@nicktrav
Copy link
Copy Markdown
Collaborator Author

TFTRs!

bors r=andrewbaptist

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 30, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-23.1.x PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only

Projects

None yet

4 participants