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

backupccl: test ignore ProtectionPolicy for exclude_data_from_backup #77406

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

adityamaru
Copy link
Contributor

@adityamaru adityamaru commented Mar 5, 2022

This change adds an end to end test to ensure that a table excluded
from backup will not holdup GC on its replica even in the presence
of a protected timestamp record covering the replica

From a users point of view, this allows them to mark a table whose
row data will be excluded from backup, and to set that tables gc.ttl
to a very low value. Backups that write PTS records will no longer
holdup GC on such low GC TTL tables.

Fixes: #73536

Release note: None

Release justification: low risk update to new functionality

@adityamaru adityamaru requested review from dt and irfansharif March 5, 2022 21:28
@adityamaru adityamaru requested a review from a team as a code owner March 5, 2022 21:28
@adityamaru adityamaru requested a review from a team March 5, 2022 21:28
@adityamaru adityamaru requested a review from a team as a code owner March 5, 2022 21:28
@cockroach-teamcity
Copy link
Member

This change is Reviewable

// TestExcludeDataFromBackupDoesNotHoldupGC tests that a table marked as
// `exclude_data_from_backup` and with a protected timestamp record covering it
// does not holdup GC, since its data is not going to be backed up.
func TestExcludeDataFromBackupDoesNotHoldupGC(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I solemnly swear to datadriven-ify these tests as part of #77129, but to derisk this change I wanted to postpone building out our backup datadriven framework. I have ideas on how to wrap all these GC/SpanConfig operations that I'll work on as part of our test revamp.

@adityamaru adityamaru force-pushed the ignore-pts-records-2 branch from d7a01a4 to 5fb8960 Compare March 8, 2022 11:44
This change adds an end to end test to ensure that a table excluded
from backup will not holdup GC on its replica even in the presence
of a protected timestamp record covering the replica

From a users point of view, this allows them to mark a table whose
row data will be excluded from backup, and to set that tables gc.ttl
to a very low value. Backups that write PTS records will no longer
holdup GC on such low GC TTL tables.

Fixes: cockroachdb#73536

Release note: None

Release justification: low risk update to new functionality
@adityamaru adityamaru force-pushed the ignore-pts-records-2 branch from 5fb8960 to 3470def Compare March 8, 2022 14:41
@adityamaru adityamaru changed the title backupccl: ignore ProtectionPolicy for exclude_data_from_backup backupccl: test ignore ProtectionPolicy for exclude_data_from_backup Mar 8, 2022
@adityamaru adityamaru removed request for a team March 8, 2022 14:42
@adityamaru
Copy link
Contributor Author

@irfansharif removing this from your review queue since this has become a test only change after #77392. This is RFAL now that that has merged!

1 similar comment
@adityamaru
Copy link
Contributor Author

@irfansharif removing this from your review queue since this has become a test only change after #77392. This is RFAL now that that has merged!

@adityamaru adityamaru removed request for irfansharif and a team March 9, 2022 03:38
@adityamaru
Copy link
Contributor Author

friendly ping @dt

@adityamaru
Copy link
Contributor Author

CI failure is TestDockerCLI

TFTR!
bors r=dt

@craig
Copy link
Contributor

craig bot commented Mar 10, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 10, 2022

Build succeeded:

@craig craig bot merged commit 8d046fc into cockroachdb:master Mar 10, 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.

sql,protectedts: mark tables as ephemeral to exclude them from backups
3 participants