Skip to content

Commit 94939c5

Browse files
committed
Fix pg_upgrade around multixid and mxoff wraparound
pg_resetwal didn't accept multixid 0 or multixact offset UINT32_MAX, but they are both valid values that can appear in the control file. That caused pg_upgrade to fail if you tried to upgrade a cluster exactly at multixid or offset wraparound, because pg_upgrade calls pg_resetwal to restore multixid/offset on the new cluster to the values from the old cluster. To fix, allow those values in pg_resetwal. Fixes bugs #18863 and #18865 reported by Dmitry Kovalenko. Backpatch down to v15. Version 14 has the same bug, but the patch doesn't apply cleanly there. It could be made to work but it doesn't seem worth the effort given how rare it is to hit this problem with pg_upgrade, and how few people are upgrading to v14 anymore. Author: Maxim Orlov <orlovmg@gmail.com> Discussion: https://www.postgresql.org/message-id/CACG%3DezaApSMTjd%3DM2Sfn5Ucuggd3FG8Z8Qte8Xq9k5-%2BRQis-g@mail.gmail.com Discussion: https://www.postgresql.org/message-id/18863-72f08858855344a2@postgresql.org Discussion: https://www.postgresql.org/message-id/18865-d4c66cf35c2a67af@postgresql.org Backpatch-through: 15
1 parent 55cefad commit 94939c5

File tree

2 files changed

+22
-15
lines changed

2 files changed

+22
-15
lines changed

src/bin/pg_resetwal/pg_resetwal.c

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,10 @@ static TransactionId set_xid = 0;
7070
static TransactionId set_oldest_commit_ts_xid = 0;
7171
static TransactionId set_newest_commit_ts_xid = 0;
7272
static Oid set_oid = 0;
73+
static bool mxid_given = false;
7374
static MultiXactId set_mxid = 0;
74-
static MultiXactOffset set_mxoff = (MultiXactOffset) -1;
75+
static bool mxoff_given = false;
76+
static MultiXactOffset set_mxoff = 0;
7577
static TimeLineID minXlogTli = 0;
7678
static XLogSegNo minXlogSegNo = 0;
7779
static int WalSegSz;
@@ -118,6 +120,7 @@ main(int argc, char *argv[])
118120
MultiXactId set_oldestmxid = 0;
119121
char *endptr;
120122
char *endptr2;
123+
int64 tmpi64;
121124
char *DataDir = NULL;
122125
char *log_fname = NULL;
123126
int fd;
@@ -254,28 +257,30 @@ main(int argc, char *argv[])
254257
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
255258
exit(1);
256259
}
257-
if (set_mxid == 0)
258-
pg_fatal("multitransaction ID (-m) must not be 0");
259260

260261
/*
261262
* XXX It'd be nice to have more sanity checks here, e.g. so
262263
* that oldest is not wrapped around w.r.t. nextMulti.
263264
*/
264265
if (set_oldestmxid == 0)
265266
pg_fatal("oldest multitransaction ID (-m) must not be 0");
267+
mxid_given = true;
266268
break;
267269

268270
case 'O':
269271
errno = 0;
270-
set_mxoff = strtoul(optarg, &endptr, 0);
272+
tmpi64 = strtoi64(optarg, &endptr, 0);
271273
if (endptr == optarg || *endptr != '\0' || errno != 0)
272274
{
273275
pg_log_error("invalid argument for option %s", "-O");
274276
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
275277
exit(1);
276278
}
277-
if (set_mxoff == -1)
278-
pg_fatal("multitransaction offset (-O) must not be -1");
279+
if (tmpi64 < 0 || tmpi64 > (int64) MaxMultiXactOffset)
280+
pg_fatal("multitransaction offset (-O) must be between 0 and %u", MaxMultiXactOffset);
281+
282+
set_mxoff = (MultiXactOffset) tmpi64;
283+
mxoff_given = true;
279284
break;
280285

281286
case 'l':
@@ -454,7 +459,7 @@ main(int argc, char *argv[])
454459
if (set_oid != 0)
455460
ControlFile.checkPointCopy.nextOid = set_oid;
456461

457-
if (set_mxid != 0)
462+
if (mxid_given)
458463
{
459464
ControlFile.checkPointCopy.nextMulti = set_mxid;
460465

@@ -464,7 +469,7 @@ main(int argc, char *argv[])
464469
ControlFile.checkPointCopy.oldestMultiDB = InvalidOid;
465470
}
466471

467-
if (set_mxoff != -1)
472+
if (mxoff_given)
468473
ControlFile.checkPointCopy.nextMultiOffset = set_mxoff;
469474

470475
if (minXlogTli > ControlFile.checkPointCopy.ThisTimeLineID)
@@ -808,7 +813,7 @@ PrintNewControlValues(void)
808813
newXlogSegNo, WalSegSz);
809814
printf(_("First log segment after reset: %s\n"), fname);
810815

811-
if (set_mxid != 0)
816+
if (mxid_given)
812817
{
813818
printf(_("NextMultiXactId: %u\n"),
814819
ControlFile.checkPointCopy.nextMulti);
@@ -818,7 +823,7 @@ PrintNewControlValues(void)
818823
ControlFile.checkPointCopy.oldestMultiDB);
819824
}
820825

821-
if (set_mxoff != -1)
826+
if (mxoff_given)
822827
{
823828
printf(_("NextMultiOffset: %u\n"),
824829
ControlFile.checkPointCopy.nextMultiOffset);

src/bin/pg_resetwal/t/001_basic.pl

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,12 @@
119119
[ 'pg_resetwal', '-m' => '10,bar', $node->data_dir ],
120120
qr/error: invalid argument for option -m/,
121121
'fails with incorrect -m option part 2');
122-
command_fails_like(
123-
[ 'pg_resetwal', '-m' => '0,10', $node->data_dir ],
124-
qr/must not be 0/,
125-
'fails with -m value 0 part 1');
122+
123+
# This used to be forbidden, but nextMulti can legitimately be 0 after
124+
# wraparound, so we now accept it in pg_resetwal too.
125+
command_ok([ 'pg_resetwal', '-m' => '0,10', $node->data_dir ],
126+
'succeeds with -m value 0 part 1');
127+
126128
command_fails_like(
127129
[ 'pg_resetwal', '-m' => '10,0', $node->data_dir ],
128130
qr/must not be 0/,
@@ -143,7 +145,7 @@
143145
'fails with incorrect -O option');
144146
command_fails_like(
145147
[ 'pg_resetwal', '-O' => '-1', $node->data_dir ],
146-
qr/must not be -1/,
148+
qr/must be between 0 and 4294967295/,
147149
'fails with -O value -1');
148150
# --wal-segsize
149151
command_fails_like(

0 commit comments

Comments
 (0)