PITR: enhance getRecordTimestamp()
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t44479psql -h localhost -U postgresBuilt from patchset v3 (message #3), July 27, 2026 at 03:56 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t44479_3 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t44479_3 && git checkout t44479_3Patchset v3 (message #3) is on t44479_3
For PITR, getRecordTimestamp() did not include all record types that
contain times.
Add handling for checkpoints, end of recovery and prepared xact record types.
Based on earlier discussions with community members.
Also, allow the option of recovery_target_use_origin_time = off (default) | on.
This allows PITR to consider whether it should use the local server
time of changes, or whether it should use the origin time on each
node. This is useful in multi-node data recovery.
This is part of a series of enhancements to PITR, in no specific order.
Passes make check and recovery testing; includes docs.
--
Simon Riggs http://www.EnterpriseDB.com/
Attachments:
pitr_enhance_getRecordTimestamp.v1.patchapplication/octet-stream; name=pitr_enhance_getRecordTimestamp.v1.patchDownload+106-15
On 30 Jun 2021, at 11:59, Simon Riggs <simon.riggs@enterprisedb.com> wrote:
For PITR, getRecordTimestamp() did not include all record types that
contain times.
Add handling for checkpoints, end of recovery and prepared xact record types.
+ <variablelist>
This breaks doc compilation, and looks like a stray tag as you want this entry
in the currently open variablelist?
--
Daniel Gustafsson https://vmware.com/
On Wed, 3 Nov 2021 at 13:28, Daniel Gustafsson <daniel@yesql.se> wrote:
On 30 Jun 2021, at 11:59, Simon Riggs <simon.riggs@enterprisedb.com> wrote:
For PITR, getRecordTimestamp() did not include all record types that
contain times.
Add handling for checkpoints, end of recovery and prepared xact record types.+ <variablelist>
This breaks doc compilation, and looks like a stray tag as you want this entry
in the currently open variablelist?
Thanks. Fixed and rebased.
--
Simon Riggs http://www.EnterpriseDB.com/
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.
--
Michael
On Thu, 27 Jan 2022 at 06:58, Michael Paquier <michael@paquier.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/