MIME-Version: 1.0
From: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Date: Wed, 12 Feb 2025 13:28:55 +0530
Message-ID: <CAMm1aWa_6u+o52r7h7G6pX-oWD0Qraf0ee17Ma50qxGS0B_Rzg@mail.gmail.com>
Subject: Address the bug in 041_checkpoint_at_promote.pl
To: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Michael Paquier <michael@paquier.xyz>
Content-Type: multipart/mixed; boundary="0000000000006f79b2062ded5242"
Archived-At: <https://www.postgresql.org/message-id/CAMm1aWa_6u%2Bo52r7h7G6pX-oWD0Qraf0ee17Ma50qxGS0B_Rzg%40mail.gmail.com>
Precedence: bulk

--0000000000006f79b2062ded5242
Content-Type: text/plain; charset="UTF-8"

While testing, I discovered an issue in 041_checkpoint_at_promote.pl.

# Wait until the previous restart point completes on the newly-promoted
# standby, checking the logs for that.
my $checkpoint_complete = 0;
foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default)
{
    if ($node_standby->log_contains("restartpoint complete"), $logstart)
    {
        $checkpoint_complete = 1;
        last;
    }
    usleep(100_000);
}
is($checkpoint_complete, 1, 'restart point has completed');

The code is intended to wait for the restart point to complete before
proceeding. However, it doesn't actually wait. Regardless of whether
the restart point completes, the loop exits after the first iteration
because the if condition always evaluates to true. This happens
because $logstart is not passed as an argument to log_contains() by
mistake. If the restart point operation is quick, this issue might not
be noticeable, which is often the case.

I've attached a patch to fix this issue. Please review and share your feedback.


Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft

--0000000000006f79b2062ded5242
Content-Type: application/octet-stream; 
	name="0001-Fix-the-bug-in-041_checkpoint_at_promote.pl.patch"
Content-Disposition: attachment; 
	filename="0001-Fix-the-bug-in-041_checkpoint_at_promote.pl.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_m71m1q870>
X-Attachment-Id: f_m71m1q870

RnJvbSBhYzFmY2M3MDMxZWY1NDcyNDViYTI5YjAwZTEwYmZlNzdhZmViNTZmIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBOaXRpbiBKYWRoYXYgPG5pdGluamFkaGF2QG1pY3Jvc29mdC5j
b20+CkRhdGU6IFdlZCwgMTIgRmViIDIwMjUgMDc6NDI6NDQgKzAwMDAKU3ViamVjdDogW1BBVENI
XSBGaXggdGhlIGJ1ZyBpbiAwNDFfY2hlY2twb2ludF9hdF9wcm9tb3RlLnBsCgpUaGUgY29kZSBp
c24ndCBmdW5jdGlvbmluZyBhcyBpbnRlbmRlZC4gSXQgc2hvdWxkIHdhaXQgZm9yIHRoZSByZXN0
YXJ0CnBvaW50IG9wZXJhdGlvbiB0byBjb21wbGV0ZSwgYnV0IGl0IGRvZXNuJ3QgZHVlIHRvIGEg
c21hbGwgbWlzdGFrZQp3aXRoIHRoZSBjbG9zaW5nIHBhcmVudGhlc2lzLiBUaGUgbG9nc3RhcnQg
dmFyaWFibGUgc2hvdWxkIGJlIHBhc3NlZAphcyB0aGUgc2Vjb25kIGFyZ3VtZW50IHRvIGxvZ19j
b250YWlucygpLiBUaGlzIHBhdGNoIGNvcnJlY3RzIHRoYXQgaXNzdWUuCi0tLQogc3JjL3Rlc3Qv
cmVjb3ZlcnkvdC8wNDFfY2hlY2twb2ludF9hdF9wcm9tb3RlLnBsIHwgMiArLQogMSBmaWxlIGNo
YW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL3Rl
c3QvcmVjb3ZlcnkvdC8wNDFfY2hlY2twb2ludF9hdF9wcm9tb3RlLnBsIGIvc3JjL3Rlc3QvcmVj
b3ZlcnkvdC8wNDFfY2hlY2twb2ludF9hdF9wcm9tb3RlLnBsCmluZGV4IGNiMzAxZDAxNDI3Li5j
YzVhYTgwYWY0MCAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvcmVjb3ZlcnkvdC8wNDFfY2hlY2twb2lu
dF9hdF9wcm9tb3RlLnBsCisrKyBiL3NyYy90ZXN0L3JlY292ZXJ5L3QvMDQxX2NoZWNrcG9pbnRf
YXRfcHJvbW90ZS5wbApAQCAtMTEwLDcgKzExMCw3IEBAICRub2RlX3N0YW5kYnktPnNhZmVfcHNx
bCgncG9zdGdyZXMnLAogbXkgJGNoZWNrcG9pbnRfY29tcGxldGUgPSAwOwogZm9yZWFjaCBteSAk
aSAoMCAuLiAxMCAqICRQb3N0Z3JlU1FMOjpUZXN0OjpVdGlsczo6dGltZW91dF9kZWZhdWx0KQog
ewotCWlmICgkbm9kZV9zdGFuZGJ5LT5sb2dfY29udGFpbnMoInJlc3RhcnRwb2ludCBjb21wbGV0
ZSIpLCAkbG9nc3RhcnQpCisJaWYgKCRub2RlX3N0YW5kYnktPmxvZ19jb250YWlucygicmVzdGFy
dHBvaW50IGNvbXBsZXRlIiwgJGxvZ3N0YXJ0KSkKIAl7CiAJCSRjaGVja3BvaW50X2NvbXBsZXRl
ID0gMTsKIAkJbGFzdDsKLS0gCjIuNDMuMAoK
--0000000000006f79b2062ded5242--