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: expand and cloudify cdc/initial-scan-rolling-restart #123924

Merged
merged 1 commit into from
May 10, 2024

Conversation

andyyang890
Copy link
Collaborator

This patch expands the cdc/initial-scan-rolling-restart test to also
test normal checkpoints (in addition to shutdown checkpoints). It also
updates the test to work on GCE so that it can run during nightly CI.

Informs #123371

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@andyyang890 andyyang890 force-pushed the repro_checkpoint_bug branch 3 times, most recently from 68bf16e to 6a8ba94 Compare May 10, 2024 04:43
This patch expands the `cdc/initial-scan-rolling-restart` test to also
test normal checkpoints (in addition to shutdown checkpoints). It also
updates the test to work on GCE so that it can run during nightly CI.

Release note: None
@andyyang890 andyyang890 force-pushed the repro_checkpoint_bug branch from 498094a to d4ef147 Compare May 10, 2024 04:50
@andyyang890 andyyang890 marked this pull request as ready for review May 10, 2024 04:51
@andyyang890 andyyang890 requested a review from a team as a code owner May 10, 2024 04:51
@andyyang890 andyyang890 requested review from DarrylWong, renatolabs, dt, rharding6373, wenyihu6 and a team and removed request for a team May 10, 2024 04:51
Copy link
Collaborator

@rharding6373 rharding6373 left a comment

Choose a reason for hiding this comment

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

:lgtm: Nice improvements! Looking forward to this running nightly.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @DarrylWong, @dt, @renatolabs, and @wenyihu6)

Copy link
Contributor

@wenyihu6 wenyihu6 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! 1 of 0 LGTMs obtained (waiting on @andyyang890, @DarrylWong, @dt, and @renatolabs)


pkg/cmd/roachtest/tests/cdc.go line 1291 at r1 (raw file):

		Name:             "cdc/initial-scan-rolling-restart",
		Owner:            registry.OwnerCDC,
		Cluster:          r.MakeClusterSpec(5),

Out of curiosity, any reasons you changed it to a 4 node cluster instead of a 5?

Copy link
Contributor

@wenyihu6 wenyihu6 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @andyyang890, @DarrylWong, @dt, and @renatolabs)

@wenyihu6 wenyihu6 self-requested a review May 10, 2024 14:42
Copy link
Contributor

@wenyihu6 wenyihu6 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @andyyang890, @DarrylWong, @dt, and @renatolabs)

Copy link
Collaborator Author

@andyyang890 andyyang890 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! 2 of 0 LGTMs obtained (waiting on @DarrylWong, @dt, @renatolabs, and @wenyihu6)


pkg/cmd/roachtest/tests/cdc.go line 1291 at r1 (raw file):

Previously, wenyihu6 (Wenyi Hu) wrote…

Out of curiosity, any reasons you changed it to a 4 node cluster instead of a 5?

Ah I made this change because in the version of the test where I don't split the ranges, it seems like usually only one node gets the aggregator so I wanted to make it more likely we'd restart that node sooner. Also I believe the default range replication factor is 3 so that's why it's 4 in total (1 node that shouldn't have replicas + 3 with replicas that we're restarting).

@andyyang890
Copy link
Collaborator Author

TFTRs!

bors r=rharding6373,wenyihu6

@andyyang890
Copy link
Collaborator Author

bors retry

@craig craig bot merged commit fd581e5 into cockroachdb:master May 10, 2024
22 checks passed
@andyyang890 andyyang890 deleted the repro_checkpoint_bug branch May 10, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants