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: refactor backup destination resolution #53060

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

pbardea
Copy link
Contributor

@pbardea pbardea commented Aug 19, 2020

During backup planning we need to resolve the backup destination. This
means that we need to:

  1. Determine if we're going to resolve to a backup collection or not
  2. Determine if we should auto-append
  3. Read any encryption options we may need if we determine that we're
    donig an incremental backup
  4. Find the previous backups in the chain if we're doing an incremental
    backup.

Release note: None

@pbardea pbardea requested review from dt, adityamaru and a team August 19, 2020 17:49
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@pbardea
Copy link
Contributor Author

pbardea commented Aug 19, 2020

This is a WIP, but ready for early feedback. I still want to rework some of the naming (e.g. encryptionContext) and need to do general clean up.

@pbardea pbardea marked this pull request as draft August 19, 2020 17:51
@pbardea pbardea force-pushed the refactor-backup-dest-resolution branch from 05175d1 to bf760c0 Compare August 19, 2020 18:17
@pbardea
Copy link
Contributor Author

pbardea commented Aug 19, 2020

I'm going to implement the TODO on resolveDests first.

@adityamaru
Copy link
Contributor

Did a first pass this LGTM, really like how it cleans up the ever 🎈-ing backupPlanHook.

@pbardea pbardea force-pushed the refactor-backup-dest-resolution branch 2 times, most recently from 8cc50ad to c5f93d5 Compare August 20, 2020 13:28
@pbardea
Copy link
Contributor Author

pbardea commented Aug 20, 2020

Cleaned it up a bit - it should be RFAL.

@pbardea pbardea marked this pull request as ready for review August 20, 2020 13:29
@pbardea pbardea force-pushed the refactor-backup-dest-resolution branch from c5f93d5 to 5af66ca Compare August 20, 2020 13:30
During backup planning we need to resolve the backup destination. This
means that we need to:
1. Determine if we're going to resolve to a backup collection or not
2. Determine if we should auto-append
3. Read any encryption options we may need if we determine that we're
donig an incremental backup
4. Find the previous backups in the chain if we're doing an incremental
backup.

Release note: None
@pbardea pbardea force-pushed the refactor-backup-dest-resolution branch from 1ce0055 to e73db28 Compare August 20, 2020 21:46
@pbardea
Copy link
Contributor Author

pbardea commented Aug 20, 2020

TFTR!
bors r=dt

@craig
Copy link
Contributor

craig bot commented Aug 20, 2020

Build succeeded:

@craig craig bot merged commit 904b7cb into cockroachdb:master Aug 20, 2020
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