From: | Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PITR: enhance getRecordTimestamp() |
Date: | 2022-01-31 19:11:39 |
Message-ID: | CANbhV-EpBT=g0UxkFnqeEhtOGyByYp26DsMEndHnnzVxsPokcg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, 27 Jan 2022 at 06:58, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Nov 03, 2021 at 04:59:04PM +0000, Simon Riggs wrote:
> > Thanks. Fixed and rebased.
>
> + if (xact_info == XLOG_XACT_PREPARE)
> + {
> + if (recoveryTargetUseOriginTime)
> + {
> + xl_xact_prepare *xlrec = (xl_xact_prepare *) XLogRecGetData(record);
> + xl_xact_parsed_prepare parsed;
> +
> + ParsePrepareRecord(XLogRecGetInfo(record),
> + xlrec,
> + &parsed);
> + *recordXtime = parsed.origin_timestamp;
> + }
> + else
> + *recordXtime = ((xl_xact_prepare *) XLogRecGetData(record))->prepared_at;
>
> As I learnt recently with ece8c76, there are cases where an origin
> timestamp may not be set in the WAL record that includes the origin
> timestamp depending on the setup done on the origin cluster. Isn't
> this code going to finish by returning true when enabling
> recovery_target_use_origin_time in some cases, even if recordXtime is
> 0? So it seems to me that this is lacking some sanity checks if
> recordXtime is 0.
>
> Could you add some tests for this proposal? This adds various PITR
> scenarios that would be uncovered, and TAP should be able to cover
> that.
Thanks. Yes, will look at that.
--
Simon Riggs http://www.EnterpriseDB.com/
From | Date | Subject | |
---|---|---|---|
Next Message | Mark Dilger | 2022-01-31 19:17:12 | Re: Granting SET and ALTER SYSTE privileges for GUCs |
Previous Message | Andrey Lepikhov | 2022-01-31 19:03:51 | Re: Multiple Query IDs for a rewritten parse tree |