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

ccl/changefeedccl/cdcevent: Fix flake in TestEventColumnOrderingWithSchemaChanges #137334

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

spilchen
Copy link
Contributor

This commit addresses a test flake in TestEventColumnOrderingWithSchemaChanges. The test relies on the SCHEMA CHANGE GC job, which triggers the RangeFeedDeleteRange event. Since this job runs asynchronously, delays in its execution occasionally caused the test to fail.

To mitigate this, the test has been updated to ensure the job starts more promptly. Additionally, debugging aids have been added to facilitate future investigations if the test fails again.

Epic: None
Closes #136371
Release note: None

@spilchen spilchen self-assigned this Dec 12, 2024
@spilchen spilchen requested a review from a team as a code owner December 12, 2024 13:54
@spilchen spilchen requested review from rharding6373 and removed request for a team December 12, 2024 13:54
@cockroach-teamcity
Copy link
Member

This change is Reviewable

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.

Thanks for the quick changes! :lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @spilchen)


pkg/ccl/changefeedccl/cdcevent/event_test.go line 739 at r1 (raw file):

	} {
		t.Run(tc.testName, func(t *testing.T) {
			log.Infof(ctx, "%s", tc.testName)

nit: this is be unnecessary. The test name will be logged by the test when it starts running (or when it's paused/resumed/succeeded/failed), like:

=== RUN   TestEventColumnOrderingWithSchemaChanges/main/main_cols_no_rewrite

pkg/ccl/changefeedccl/cdcevent/event_test.go line 741 at r1 (raw file):

			log.Infof(ctx, "%s", tc.testName)
			sqlDB.ExecMultiple(t,
				`DROP TABLE IF EXISTS foo`,

Nice, thanks!

…chemaChanges

This commit addresses a test flake in
TestEventColumnOrderingWithSchemaChanges. The test relies on the SCHEMA
CHANGE GC job, which triggers the RangeFeedDeleteRange event. Since this
job runs asynchronously, delays in its execution occasionally caused the
test to fail.

To mitigate this, the test has been updated to ensure the job starts
more promptly. Additionally, debugging aids have been added to
facilitate future investigations if the test fails again.

Epic: None
Closes cockroachdb#136371
Release note: None
@spilchen spilchen force-pushed the gh-136371/241212/0802/cdc-tc-fix branch from b573786 to cffa888 Compare December 12, 2024 17:45
Copy link
Contributor Author

@spilchen spilchen 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)


pkg/ccl/changefeedccl/cdcevent/event_test.go line 739 at r1 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

nit: this is be unnecessary. The test name will be logged by the test when it starts running (or when it's paused/resumed/succeeded/failed), like:

=== RUN   TestEventColumnOrderingWithSchemaChanges/main/main_cols_no_rewrite

Makes sense.

@spilchen
Copy link
Contributor Author

TFTR!

bors r+

@craig craig bot merged commit 497656e into cockroachdb:master Dec 12, 2024
22 checks passed
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.

ccl/changefeedccl/cdcevent: TestEventColumnOrderingWithSchemaChanges failed
3 participants