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

roachtest: prevent shared mutable state across c2c roachtest runs #101220

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

msbutler
Copy link
Collaborator

Previously, all c2c/* roachtests run with --count would provide incomprehensible results because multiple roachtest runs of the same test would override each other's state. Specifically, the latest call of test_spec.Run(), would override the test.Test harness, and syncedCluster.Cluster used by all other tests with the same registration.

This patch fixes this problem by moving all fields in replicationSpec that are set during test execution (i.e. a test_spec.Run call), to a new replicationDriver struct. Now, replicationSpec gets defined during test registration and is shared across test runs, while replicationDriver gets set within a test run.

Epic: None
Release note: None

Previously, all `c2c/*` roachtests run with `--count` would provide
incomprehensible results because multiple roachtest runs of the same test would
override each other's state. Specifically, the latest call of
`test_spec.Run()`, would override the `test.Test` harness, and
`syncedCluster.Cluster` used by all other tests with the same registration.

This patch fixes this problem by moving all fields in `replicationSpec` that are
set during test execution (i.e. a `test_spec.Run` call), to a new
`replicationDriver` struct. Now, `replicationSpec` gets defined during test
registration and is shared across test runs, while `replicationDriver`
gets set within a test run.

Epic: None
Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@msbutler msbutler marked this pull request as ready for review April 11, 2023 15:02
@msbutler msbutler requested a review from a team as a code owner April 11, 2023 15:02
@msbutler msbutler requested review from srosenberg, smg260 and benbardin and removed request for a team April 11, 2023 15:02
@msbutler
Copy link
Collaborator Author

TFTR!

bors r=benbardin

@msbutler
Copy link
Collaborator Author

bors r=benbardin

@craig
Copy link
Contributor

craig bot commented Apr 11, 2023

Build succeeded:

@craig craig bot merged commit 1abff58 into cockroachdb:master Apr 11, 2023
craig bot pushed a commit that referenced this pull request Apr 14, 2023
101447: c2c: randomize node shutdown timing in roachtests r=stevendanna a=msbutler

Previously, the c2c/nodeShutdown tests would only execute a shutdown after a high water mark was set and before a cutover timestamp was chosen. This patch randomizes the node shutdown to either occur during the initial scan, during this steady state phase, or during cutover.

This patch also fixes an infra bug that caused /src shutdown tests to run on /dest and /coordinator tests to run on /worker introduced in #101220.

Informs: #89487

Release note: none

Co-authored-by: Michael Butler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants