zalando / spilo

Highly available elephant herd: HA PostgreSQL cluster using Docker
Apache License 2.0
1.52k stars 374 forks source link

Improved Handling of `DAYS_TO_RETAIN` in Backup Script #938

Open a-thomas-22 opened 9 months ago

a-thomas-22 commented 9 months ago

Description:

This PR focuses on the refinement and enhancement of the DAYS_TO_RETAIN logic within the backup script. The primary objective is to establish clearer, more explicit behavior while ensuring backwards compatibility for any deployments that rely on existing behaviors.

Key Changes:

  1. Explicit DAYS_TO_RETAIN Handling:

    • The way the script determines the value of DAYS_TO_RETAIN has been made more explicit. If not set externally, it falls back to the value of BACKUP_NUM_TO_RETAIN.
    • Log messages have been introduced to notify when DAYS_TO_RETAIN is sourced from BACKUP_NUM_TO_RETAIN, ensuring transparency in its determination.
  2. Improved Backup Deletion Logic:

    • The logic which determines which backups are old enough for deletion based on DAYS_TO_RETAIN has been refined for clarity, accompanied by more descriptive log messages.
  3. Backwards Compatibility:

    • For deployments that might be relying on the previous behavior where BACKUP_NUM_TO_RETAIN was implicitly used, this fallback mechanism remains intact. This ensures that the script won't break or change behavior unexpectedly for existing setups.
    • Newer deployments or those that explicitly set DAYS_TO_RETAIN will benefit from the clearer, more descriptive behavior and logging.

Reason for Change:

The primary reason for this change is to enhance clarity and predictability around backup retention durations. Fixes #937

a-thomas-22 commented 1 month ago

I made a sidecar workaround for kubernetes until a PR to fix is made for those of us having this issue https://github.com/a-thomas-22/wal-g-pruner