Watch interval validation fix #569
Conversation
|
On the side note --watch-backup-name-template seems to have some problems too. Pattern "ch_{replica}-{type}-{time:20060102150405}" results in default |
| @@ -53,7 +53,7 @@ func (b *Backuper) ValidateWatchParams(watchInterval, fullInterval, watchBackupN | |||
| if watchBackupNameTemplate != "" { | |||
| b.cfg.General.WatchBackupNameTemplate = watchBackupNameTemplate | |||
| } | |||
| if b.cfg.General.FullDuration.Seconds() < b.cfg.General.WatchDuration.Seconds()*float64(b.cfg.General.BackupsToKeepRemote) { | |||
| if b.cfg.General.FullDuration.Seconds() > b.cfg.General.WatchDuration.Seconds()*float64(b.cfg.General.BackupsToKeepRemote) { | |||
There was a problem hiding this comment.
are you sure this will help resolve your "connection close" issue?
There was a problem hiding this comment.
Sorry, I don't understand what exactly you try to fix
this part of code is not related to current validation allow backups_to_keep_remote less than 4
could you provide more examples in comments?
There was a problem hiding this comment.
Sure.
Current (wrong) version in pseudo-code:
If FullDurationSeconds < (BackupsToKeep * WatchInterval) {
raise validation error
}
It prevents user from increasing amount of backups - if we set BackupsToKeep to infinity validation starts failing.
Example:
Let say we want to make incremental daily backups and have full backup on Sunday and to store two weeks of backups on remote.
FullDurationSeconds = 1 week (604800 seconds)
BackupsToKeep = 14
WatchInterval = 1 day (86400 seconds)
Validation will fail, since 604800 < (14 * 86400), it would only work if BackupsToKeep is less than 7, which is not enough to store a full week. With the changes it will accept any value of BackupsToKeep larger than 7.
There was a problem hiding this comment.
ok. got it
could you fix intergration_test.go for https://github.com/AlexAkulov/clickhouse-backup/actions/runs/3541456947/jobs/5949819982#step:7:56324
look like your changes broke some backup API scenarios
There was a problem hiding this comment.
Tests on my machine are ridiculously slow, something is not right
|
oops sorry, miss you with author of #568 |
| @@ -53,7 +53,7 @@ func (b *Backuper) ValidateWatchParams(watchInterval, fullInterval, watchBackupN | |||
| if watchBackupNameTemplate != "" { | |||
| b.cfg.General.WatchBackupNameTemplate = watchBackupNameTemplate | |||
| } | |||
| if b.cfg.General.FullDuration.Seconds() < b.cfg.General.WatchDuration.Seconds()*float64(b.cfg.General.BackupsToKeepRemote) { | |||
| if b.cfg.General.FullDuration.Seconds() > b.cfg.General.WatchDuration.Seconds()*float64(b.cfg.General.BackupsToKeepRemote) { | |||
There was a problem hiding this comment.
Sorry, I don't understand what exactly you try to fix
this part of code is not related to current validation allow backups_to_keep_remote less than 4
could you provide more examples in comments?
Current validation prevents from properly using watch functionality as it requires a wrong number of stored backups to work.
Example:
1h watch interval, 5h full interval needs at least 5 stored remote backups to work, but validation only accepts 4 or less.