backupccl: Add ALTER BACKUP SCHEDULE command. #85489
Conversation
|
This change is |
|
Ready for another look, thank you! |
benbardin
force-pushed
the
as3
branch
2 times, most recently
from
19fb42d
to
d20bbda
Compare
| func loadSchedules( | ||
| ctx context.Context, p sql.PlanHookState, eval *alterBackupScheduleEval, | ||
| ) ( | ||
| *jobs.ScheduledJob, |
There was a problem hiding this comment.
nit: comments, or named returns, could be helpful here given the multiple returns of the same type. or maybe a little struct with fullJob... incJob, incArgs or something vs the 4 returns? 🤷
| # Disable incremental backup. | ||
|
|
||
| exec-sql | ||
| alter backup schedule $fullID set full backup always; |
There was a problem hiding this comment.
Is there a test that both sets full backup always and changes the recurrence (eg. to daily)?
|
|
||
| // We don't know if a full backup has completed yet, so pause incremental | ||
| // until a full backup completes. | ||
| incJob.Pause() |
There was a problem hiding this comment.
Huh, I'm not sure we want to pause it here? like, say we do 10min inc and we were doing daily full but need to switch to 6h full since the chains are getting long; we do alter ... full backup */6 * ... etc, and then I think we expect our next full to run at 1800 instead of 0000, but we probably expect our inc backups to carry on on their 10min cadence as if nothing changed, no?
There was a problem hiding this comment.
We can only get to this branch if incJob == nil, that is, we were doing no incremental backups of any kind before, but we're adding some now. I think PAUSE is right in that case, no? We could do fancy logic to see if the FULL schedule has been run before, I suppose, but this does seem simpler and quite reasonable.
Note that only SET RECURRING and SET FULL BACKUP are supported here. I'll add the other commands shortly. Release note (enterprise change): Add ALTER BACKUP SCHEDULE command to modify existing backup schedules.
|
Tests are green, RFAL! Thanks! |
|
bors r+ |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
Note that only SET RECURRING and SET FULL BACKUP are supported here. I'll add
the other commands shortly.
Release note (enterprise change): Add ALTER BACKUP SCHEDULE command to modify
existing backup schedules.
Partially addresses #84033