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

changefeedccl: verify changefeed failure for reverted import #89169

Merged

Conversation

samiskin
Copy link
Contributor

@samiskin samiskin commented Oct 3, 2022

Resolves #82943

A new test verifies that even if an import was reverted the changefeed still fails on a table offline error. This is now needed to ensure we don't have undesired behavior during a case where rangefeeds would emit range tombstones.

Release note: None

@samiskin samiskin requested a review from a team as a code owner October 3, 2022 12:13
@samiskin samiskin requested review from miretskiy and removed request for a team October 3, 2022 12:13
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@miretskiy miretskiy left a comment

Choose a reason for hiding this comment

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

Nice.

sqlDB.Exec(t, "INSERT INTO for_import VALUES (0, 10);")

// Start an import job which will immediately pause after ingestion
sqlDB.Exec(t, "SET CLUSTER SETTING jobs.debug.pausepoints = 'import.after_ingest';")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice (@dt would be trilled to see this).

cdcTestNamed(t, "reverted import fails changefeed with earlier cursor", func(t *testing.T, s TestServer, f cdctest.TestFeedFactory) {
sqlDB := sqlutils.MakeSQLRunner(s.DB)
sqlDB.Exec(t, "SET CLUSTER SETTING kv.bulk_io_write.small_write_size = '1';")
sqlDB.Exec(t, "SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true;")
Copy link
Contributor Author

@samiskin samiskin Oct 3, 2022

Choose a reason for hiding this comment

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

@miretskiy I assume the ConstantWithMetamorphicTestBool on the cluster setting is something that applies to every test, so I'm wary of making a change like that here just for this test.

Copy link
Contributor

Choose a reason for hiding this comment

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

ack

@samiskin samiskin force-pushed the changefeed-tombstone-ignore-test branch 2 times, most recently from 0dd0569 to f27f1f9 Compare October 3, 2022 14:24
Resolves cockroachdb#82943

A new test verifies that even if an import was reverted the changefeed
still fails on a table offline error.  This is now needed to ensure we
don't have undesired behavior during a case where  rangefeeds would emit
range tombstones.

Release note: None
@samiskin samiskin force-pushed the changefeed-tombstone-ignore-test branch from f27f1f9 to 29ce5b1 Compare October 3, 2022 18:59
@samiskin
Copy link
Contributor Author

samiskin commented Oct 3, 2022

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 3, 2022

Build succeeded:

@craig craig bot merged commit fca6c42 into cockroachdb:master Oct 3, 2022
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.

changefeedccl: verify MVCC range tombstones are never visible to CDC
3 participants