Content-Type: multipart/mixed; boundary="------------ZoWItoPiIj0nPe0KRx0MNJOy"
Message-ID: <f3ce2134-d72b-489b-aba5-f8c012c7708e@postgrespro.ru>
Date: Wed, 17 Jun 2026 07:53:12 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: 004_timeline_switch TAP test may fail
To: Ewan Young <kdbase.hack@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
References: <fea4190e-f8b5-4432-a52d-bcbee5f34366@postgrespro.ru>
 <CAON2xHMO2EVsjR+6ab+w6BW5ycyveK5UD5nCik+Uc1b=G6pXiA@mail.gmail.com>
Content-Language: en-US
From: Sergey Tatarintsev <s.tatarintsev@postgrespro.ru>
In-Reply-To: 
 <CAON2xHMO2EVsjR+6ab+w6BW5ycyveK5UD5nCik+Uc1b=G6pXiA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/f3ce2134-d72b-489b-aba5-f8c012c7708e%40postgrespro.ru>
Precedence: bulk

This is a multi-part message in MIME format.
--------------ZoWItoPiIj0nPe0KRx0MNJOy
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi!
Thanks for review!
v2 patch attached
comment changed, count(1) replaced with count(*).


> Hi Sergey,
>
> Thanks for the report and patch. I think the analysis is right, and the
> fix is in the right place.
>
> The gap traces back to commit 7185eddf, which deliberately dropped the
> wait_for_catchup() and switched the primary from teardown_node() to a
> clean stop(), on the grounds that a clean stop flushes all WAL to both
> standbys before exiting. That's true, but only for standbys whose
> walsender is *connected* at shutdown time -- and ->start() only waits
> for the postmaster to accept connections, not for the standby's
> walreceiver to have connected back to the primary. So if a standby
> hasn't connected yet when the primary stops, the clean-shutdown flush
> skips it, and we're back to the exact "standbys received different
> amounts of WAL -> timeline fork on reconnect" failure that 7185eddf was
> meant to fix.
>
> Polling pg_stat_replication until both walsenders are present closes
> that hole: it re-establishes the precondition the clean-stop design
> silently assumed. And connection is enough here -- the walsender
> shutdown path sends all WAL up to the shutdown checkpoint regardless of
> catchup state -- so there's no need to additionally check
> state = 'streaming'.
>
> One small thing: the rest of this file uses count(*), so I'd write count(*) = 2
> rather than count(1) = 2 just for local consistency. And the comment reads a
> little better as something like "Wait until both standbys have
> connected to the primary",
> since by this point they've already started -- what we're waiting for is the
> connection.
>
> Regards,
> Ewan
>
> On Tue, Jun 16, 2026 at 4:01 PM Sergey Tatarintsev
> <s.tatarintsev@postgrespro.ru> wrote:
>> Hi hackers!
>>
>> I found that after commit 7185eddf0522b3146ed1ff6e063e8e129e77c706 we
>> got little omission
>> in TAP test 004_timeline_switch:
>> ...
>> my $node_standby_1 = PostgreSQL::Test::Cluster->new('standby_1');
>> ...
>> $node_primary->stop;
>>
>> There is no guarantee that standby_1 and standby_2 was successfully
>> connected to primary and start
>> streaming before primary stopped.
>>
>>    I think we must ensure that primary knows about standby_1 and standby_2
>>
>> --
>> With best regards,
>> Sergey Tatarintsev,
>> PostgresPro


-- 
With best regards,
Sergey Tatarintsev,
PostgresPro

--------------ZoWItoPiIj0nPe0KRx0MNJOy
Content-Type: text/x-patch; charset=UTF-8;
 name="v2-0001-Fix-004_timeline_switch-TAP-test-wait-for-standbys-s.patch"
Content-Disposition: attachment;
 filename*0="v2-0001-Fix-004_timeline_switch-TAP-test-wait-for-standbys-s";
 filename*1=".patch"
Content-Transfer-Encoding: base64

RnJvbSBkZDQ0OTM5NjA2MGM3MWMzNGJjYjAzZTVjMWM1ZGUwY2IwODY4ZGEwIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBTZXJnZXkgVGF0YXJpbnRzZXYgPHMudGF0YXJpbnRz
ZXZAcG9zdGdyZXNwcm8ucnU+CkRhdGU6IFR1ZSwgMTYgSnVuIDIwMjYgMTE6NTc6MzkgKzA3
MDAKU3ViamVjdDogW1BBVENIXSBGaXggMDA0X3RpbWVsaW5lX3N3aXRjaCBUQVAgdGVzdDog
d2FpdCBmb3Igc3RhbmRieXMgc3RhcnRzCiBiZWZvcmUgcHJpbWFyeSBzdG9wcwoKLS0tCiBz
cmMvdGVzdC9yZWNvdmVyeS90LzAwNF90aW1lbGluZV9zd2l0Y2gucGwgfCA0ICsrKysKIDEg
ZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKykKCmRpZmYgLS1naXQgYS9zcmMvdGVzdC9y
ZWNvdmVyeS90LzAwNF90aW1lbGluZV9zd2l0Y2gucGwgYi9zcmMvdGVzdC9yZWNvdmVyeS90
LzAwNF90aW1lbGluZV9zd2l0Y2gucGwKaW5kZXggZTBiMzg1MTkyN2MuLmM4N2YwNzljZWY4
IDEwMDY0NAotLS0gYS9zcmMvdGVzdC9yZWNvdmVyeS90LzAwNF90aW1lbGluZV9zd2l0Y2gu
cGwKKysrIGIvc3JjL3Rlc3QvcmVjb3ZlcnkvdC8wMDRfdGltZWxpbmVfc3dpdGNoLnBsCkBA
IC0zMCw2ICszMCwxMCBAQCAkbm9kZV9zdGFuZGJ5XzItPmluaXRfZnJvbV9iYWNrdXAoJG5v
ZGVfcHJpbWFyeSwgJGJhY2t1cF9uYW1lLAogCWhhc19zdHJlYW1pbmcgPT4gMSk7CiAkbm9k
ZV9zdGFuZGJ5XzItPnN0YXJ0OwogCisjIFdhaXQgdW50aWwgYm90aCBzdGFuZGJ5cyBoYXZl
IGNvbm5lY3RlZCB0byB0aGUgcHJpbWFyeQorJG5vZGVfcHJpbWFyeS0+cG9sbF9xdWVyeV91
bnRpbCgncG9zdGdyZXMnLAorCSJTRUxFQ1QgY291bnQoKikgPSAyIEZST00gcGdfc3RhdF9y
ZXBsaWNhdGlvbiIpOworCiAjIENyZWF0ZSBzb21lIGNvbnRlbnQgb24gcHJpbWFyeQogJG5v
ZGVfcHJpbWFyeS0+c2FmZV9wc3FsKCdwb3N0Z3JlcycsCiAJIkNSRUFURSBUQUJMRSB0YWJf
aW50IEFTIFNFTEVDVCBnZW5lcmF0ZV9zZXJpZXMoMSwxMDAwKSBBUyBhIik7Ci0tIAoyLjQz
LjAKCg==

--------------ZoWItoPiIj0nPe0KRx0MNJOy--