Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/299 PIT backups #301

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Feature/299 PIT backups #301

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9746f2a
fixed the replace tzinfo for last_modified
kidynamit Feb 7, 2019
06b5057
Added expanded_size_bytes for PIT backup
kidynamit Nov 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions postgres-appliance/bootstrap/clone_with_wale.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def choose_backup(output, recovery_target_time):
match_timestamp = match = None
for backup in backup_list:
last_modified = parse(backup['last_modified'])
last_modified = last_modified.replace(tzinfo=recovery_target_time.tzinfo)
Copy link
Contributor

@CyberDem0n CyberDem0n Feb 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all I don't really understand how you managed to get such exception.

  1. Spilo is using UTC timezone.
  2. wal-e backup-list produces last_modified in the format 2019-02-09T01:01:36.000Z
  3. wal-g backup-list produces last_modified in the format 2019-02-09T01:01:36Z

All aforementioned facts will produce datetime objects with tzinfo either tzlocal or tzutc (for AWS S3).
You are changing the timezone in the last_modified timestamp from wal-e/wal-g backup-list output to a timezone from --recovery-target-time. This is undesirable.

Can it be that you passed the CLONE_TARGET_TIME without specifying a timezone?
If you are using GCS could you verify the timestamp format in the backup-list output? Does it have a timezone? If it doesn't, how it relates to UTC? Is it possible to convert it to UTC?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are changing the timezone in the last_modified timestamp from wal-e/wal-g backup-list output to a timezone from --recovery-target-time.

That is one of putting it. Another way of putting it is that I am matching the last modified timestamp to match the timezone of the recovery target timestamp for the PIT recovery to work.

Can it be that you passed the CLONE_TARGET_TIME without specifying a timezone?

Refer to issue #299 where I do specify the command I run and particularly the --recovery-target-time flag is set with the timezone specified.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is one of putting it.

The problem is that for us the last_modified already contains UTC timezone! Therefore it is really important to answer following questions:

  • If you are using GCS could you verify the timestamp format in the backup-list output?
  • Does it (timestamp) have a timezone?
  • If it doesn't, how it relates to UTC?
  • Is it possible to convert it to UTC?`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will attempt to perform the same operation with the CLONE_TARGET_TIME set. Then get back to you.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using GCS and am having the same problem as @kidynamit. The last_modified is a datatime object without tzinfo and thus it seems to be a bug in wal-e as they mention in their docs that last_modified should have tzinfo associated with it.

As for your questions @CyberDem0n

  • Format is yyyy-mm-dd hh:mm:ss
  • No, it does not have a timezone
  • Currently, there seems to be no way to relate it to UTC
  • See above, so no.

Maybe we could detect whether last_modified has a tzinfo and on None replace it with UTC? Currently this blocks me from using the Kubernetes Operator in combination with GCS for PITR restores using WAL-E and GCS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nielsmeima I've posted a fix. Turns out there was an expanded size bytes we could use.

if last_modified < recovery_target_time:
if match is None or last_modified > match_timestamp:
match = backup
Expand Down