K8SPG-870 Use delta as size for incr and diff backups#1696
K8SPG-870 Use delta as size for incr and diff backups#1696oksana-grishchenko wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates how the operator reports backup size in PerconaPGBackup status by using pgBackRest’s delta value so incremental/differential backups no longer show the full backup size.
Changes:
- Extend the pgBackRest
infoJSON parsing struct to includeinfo.delta. - Update PGBackup status reconciliation to store
backup.Info.Deltaas the reportedStatus.Size.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| percona/pgbackrest/pgbackrest.go | Adds delta to the parsed pgBackRest backup info payload. |
| percona/controller/pgbackup/controller.go | Switches PGBackup status size calculation from info.size to info.delta. |
| Type v2.PGBackupType `json:"type,omitempty"` | ||
| Info struct { | ||
| Size int64 `json:"size,omitempty"` | ||
| Size int64 `json:"size,omitempty"` |
There was a problem hiding this comment.
right, now it can be safely deleted. Updated, thanks
| } | ||
|
|
||
| if pgBackup.Status.BackupName != backup.Label || pgBackup.Status.BackupType != backup.Type || pgBackup.Status.Size != backup.Info.Size { | ||
| if pgBackup.Status.BackupName != backup.Label || pgBackup.Status.BackupType != backup.Type || pgBackup.Status.Size != backup.Info.Delta { |
There was a problem hiding this comment.
lets try to see if we can capture this part through a unit test and assert the status value and its population
There was a problem hiding this comment.
added unit-tests to check status while different conditions take place.
we also have a basic e2e test that checks the size is populated
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
percona/controller/pgbackup/controller.go:540
-
- Problem: The behavior change to populate
pgBackup.Status.Sizefrombackup.Info.Deltaisn’t covered by a unit test that exercisesupdatePGBackrestInfo(onlyfindBackupInPGBackrestInfois tested).
- Problem: The behavior change to populate
- Why it matters: This is user-visible status data; without a direct test, a refactor could accidentally stop propagating
deltato.status.size. - Fix: Add a focused unit test for
updatePGBackrestInfo(or extract a small helper likeapplyBackupStatus(pgBackup, backup)and test that it setsStatus.Size = backup.Info.Delta).
if pgBackup.Status.BackupName != backup.Label || pgBackup.Status.BackupType != backup.Type || pgBackup.Status.Size != backup.Info.Delta {
if err := pgBackup.UpdateStatus(ctx, c, func(bcp *v2.PerconaPGBackup) {
bcp.Status.BackupName = backup.Label
bcp.Status.BackupType = backup.Type
bcp.Status.Size = backup.Info.Delta
| Type v2.PGBackupType `json:"type,omitempty"` | ||
| Info struct { | ||
| Size int64 `json:"size,omitempty"` | ||
| Delta int64 `json:"delta,omitempty"` |
There was a problem hiding this comment.
adding a comment about delta being equal to size for full backups might be helpful for future readers of code
commit: 9e91f9b |
K8SPG-870
Problem:
We were reporting
info.sizethat is the entire size of the backup, even for incremental/differential.Solution:
Use
deltafor incremental/differential. For full backupsdeltais the same assizeso we can usedeltafor all types of backups.CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability