Fix portability issues with parsing of recovery_target_xid

The parsing of this parameter has been using strtoul(), which is not
portable across platforms.  On most Unix platforms, unsigned long has a
size of 64 bits, while on Windows it is 32 bits.  It is common in
recovery scenarios to rely on the output of txid_current() or even the
newer pg_current_xact_id() to get a transaction ID for setting up
recovery_target_xid.  The value returned by those functions includes the
epoch in the computed result, which would cause strtoul() to fail where
unsigned long has a size of 32 bits once the epoch is incremented.

WAL records and 2PC data include only information about 32-bit XIDs and
it is not possible to have XIDs across more than one epoch, so
discarding the high bits from the transaction ID set has no impact on
recovery.  On the contrary, the use of strtoul() prevents a consistent
behavior across platforms depending on the size of unsigned long.

This commit changes the parsing of recovery_target_xid to use
pg_strtouint64() instead, available down to 9.6.  There is one TAP test
stressing recovery with recovery_target_xid, where a tweak based on
pg_reset{xlog,wal} is added to bump the XID epoch so as this change gets
tested, as per an idea from Alexander Lakhin.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16780-107fd0c0385b1035@postgresql.org
Backpatch-through: 9.6

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a3309beebcf04fe94fde12b52eaaa893a5492670..ba2fbdcf02a423c44fdc8b09fb0c21134b6c2d31 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5341,7 +5341,7 @@ readRecoveryCommandFile(void)
        else if (strcmp(item->name, "recovery_target_xid") == 0)
        {
            errno = 0;
-           recoveryTargetXid = (TransactionId) strtoul(item->value, NULL, 0);
+           recoveryTargetXid = (TransactionId) pg_strtouint64(item->value, NULL, 0);
            if (errno == EINVAL || errno == ERANGE)
                ereport(FATAL,
                        (errcode(ERRCODE_INVALID_PARAMETER_VALUE),

diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 824fa4da529f5e71b1418d936c2eca3b8491b676..aff5402226d3de9d01131819eb42dab8cc8a7a25 100644 (file)
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -47,6 +47,10 @@ sub test_recovery_standby
 my $node_master = get_new_node('master');
 $node_master->init(has_archiving => 1, allows_streaming => 1);
 
+# Bump the transaction ID epoch.  This is useful to stress the portability
+# of recovery_target_xid parsing.
+system_or_bail('pg_resetwal', '-e', '1', $node_master->data_dir);
+
 # Start it
 $node_master->start;