APPS-1417 Respect scheduled full backup timing #291
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #291 +/- ##
==========================================
- Coverage 43.95% 43.92% -0.04%
==========================================
Files 52 52
Lines 2855 2857 +2
==========================================
Hits 1255 1255
- Misses 1474 1476 +2
Partials 126 126 ☔ View full report in Codecov by Sentry. |
|
In scheduleFullBackup I removed running backup now.
Also, I added last run to CurrentBackup stats. |
pkg/model/last_run.go
Outdated
| return r.Full == nil | ||
| } | ||
|
|
||
| func (r *LastBackupRun) LastAnyRun() *time.Time { |
pkg/model/last_run.go
Outdated
| @@ -158,7 +172,7 @@ func (h *BackupRoutineHandler) runFullBackupInternal(ctx context.Context, now ti | |||
| return err | |||
| } | |||
|
|
|||
| h.lastRun.full = now | |||
| h.lastRun.Full = &now | |||
There was a problem hiding this comment.
It is not a new thing, but it is better to synchronize writes and reads to prevent data races.
There was a problem hiding this comment.
No backup of the same type for the same routine will run simultaneously, no need for lock
There was a problem hiding this comment.
There are two jobs scheduled for the full and incremental backup that may concurrently access those variables.
| @@ -328,7 +342,7 @@ func (h *BackupRoutineHandler) runIncrementalBackupInternal(ctx context.Context, | |||
| return err | |||
| } | |||
|
|
|||
| h.lastRun.incremental = now | |||
| h.lastRun.Incremental = &now | |||
There was a problem hiding this comment.
No backup of the same type for the same routine will run simultaneously, no need for lock
pkg/model/current_backup.go
Outdated
| @@ -10,6 +10,8 @@ type CurrentBackups struct { | |||
| Full *RunningJob | |||
| // Incremental represents the state of an incremental backup. Nil if no incremental backup is running. | |||
| Incremental *RunningJob | |||
| // LastRunTime: the last time when a backup was run | |||
There was a problem hiding this comment.
LastRunTime contains information about the latest run time for both full and incremental backups.
pkg/model/last_run.go
Outdated
| Incremental *time.Time | ||
| } | ||
|
|
||
| func NewLastRun(lastFullBackup *time.Time, lastIncrBackup *time.Time) LastBackupRun { |
There was a problem hiding this comment.
Consider renaming to MakeLastBackupRun or CreateLastBackupRun, since it is not a good practice to return a value for New constructors.
reugn
changed the title
No description provided.