Content-Type: multipart/mixed; boundary="------------J009g61eu3rZ9IV0rz0SMH0s"
Message-ID: <213d107f-6c39-c257-040b-df8773fd0064@gmail.com>
Date: Mon, 8 May 2023 10:14:37 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.10.1
Subject: Re: Add two missing tests in 035_standby_logical_decoding.pl
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, "Yu Shi (Fujitsu)"
 <shiy.fnst@fujitsu.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <2fefa454-5a70-2174-ddbf-4a0e41537139@gmail.com>
 <CAA4eK1La2uVZdSNdp_AwpxVP85SvWHMT3ecfJgVYqPMjGVViyw@mail.gmail.com>
 <6711e200-0d5c-98c3-ba39-7d64a6936f7a@gmail.com>
 <CAA4eK1LQmnSroXZcR+w1JuHsOyQLdj=-QWUw8ExYem3kUpBE3A@mail.gmail.com>
 <a3f02689-fd8b-e290-2e62-9f7a5eb6f4d8@gmail.com>
 <CAA4eK1+-H9TBSG-qEd7aykdKHhRo3YCsQ_i6mquTdiL2zXVAPw@mail.gmail.com>
 <2c544b9f-419d-d18e-47be-f3d6f3d24e4e@gmail.com>
 <CAA4eK1KvDAgQC9kzHn41MJBmk-7X226-gBXVDFNubtr7rnSu6A@mail.gmail.com>
 <a155f702-a9b0-d473-0c44-06287af600b9@gmail.com>
 <CAA4eK1KhmUopfMDR+0ZkBSGerXet1XWGyguhE=ZwV5ZUUcQt7Q@mail.gmail.com>
 <3576f684-718d-ddd6-f4b0-f8ad64ad3330@gmail.com>
 <CAA4eK1+-uuH+sC42_=wA=xPyzCN+U3VNeW_d-BRGvt-pGkNT+Q@mail.gmail.com>
 <2894687f-141d-85a9-e8c3-2151e5b7e662@gmail.com>
 <CAA4eK1KD4pmEjqPkPjNdB5rTegJftzJjsZ3xuBndbPANQrueEw@mail.gmail.com>
 <7e5f41be-ec0f-b6f2-2e2a-60fd5648ee84@gmail.com>
 <CAA4eK1KYtxsvOFaG0C8fMtWFQ5YAdo9dtC4si1UuUccGSYTzCQ@mail.gmail.com>
Content-Language: en-US
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
In-Reply-To: 
 <CAA4eK1KYtxsvOFaG0C8fMtWFQ5YAdo9dtC4si1UuUccGSYTzCQ@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/213d107f-6c39-c257-040b-df8773fd0064%40gmail.com>
Precedence: bulk

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

Hi,

On 5/8/23 4:42 AM, Amit Kapila wrote:
> On Sat, May 6, 2023 at 9:33 PM Drouvot, Bertrand
> <bertranddrouvot.pg@gmail.com> wrote:
>>
>> On 5/6/23 3:28 PM, Amit Kapila wrote:
>>> On Sat, May 6, 2023 at 1:52 PM Drouvot, Bertrand
>>> <bertranddrouvot.pg@gmail.com> wrote:
>>
>>> Next steps:
>>> =========
>>> 1. The first thing is we should verify this theory by adding some LOG
>>> in KeepLogSeg() to see if the _logSegNo is reduced due to the value
>>> returned by XLogGetReplicationSlotMinimumLSN().
>>
>> Yeah, will do that early next week.

It's done with the following changes:

https://github.com/bdrouvot/postgres/commit/79e1bd9ab429a22f876b9364eb8a0da2dacaaef7#diff-c1cb3ab2a19606390c1a7ed00ffe5a45531702ca5faf999d401c548f8951c65bL7454-R7514

With that in place, there is one failing test here: https://cirrus-ci.com/task/5173216310722560

Where we can see:

2023-05-08 07:42:56.301 UTC [18038][checkpointer] LOCATION:  UpdateMinRecoveryPoint, xlog.c:2500
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOG:  00000: CreateRestartPoint: After XLByteToSeg RedoRecPtr is 0/4000098, _logSegNo is 4
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOCATION:  CreateRestartPoint, xlog.c:7271
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOG:  00000: KeepLogSeg: segno changed to 4 due to XLByteToSeg
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOCATION:  KeepLogSeg, xlog.c:7473
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOG:  00000: KeepLogSeg: segno changed to 3 due to XLogGetReplicationSlotMinimumLSN()
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOCATION:  KeepLogSeg, xlog.c:7483
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOG:  00000: CreateRestartPoint: After KeepLogSeg RedoRecPtr is 0/4000098, endptr is 0/4000148, _logSegNo is 3
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOCATION:  CreateRestartPoint, xlog.c:7284
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOG:  00000: BDT1 about to call RemoveOldXlogFiles in CreateRestartPoint
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOCATION:  CreateRestartPoint, xlog.c:7313
2023-05-08 07:42:56.302 UTC [18038][checkpointer] LOG:  00000: attempting to remove WAL segments older than log file 000000000000000000000002

So the suspicion about XLogGetReplicationSlotMinimumLSN() was correct (_logSegNo moved from
4 to 3 due to XLogGetReplicationSlotMinimumLSN()).

>> What about postponing the physical slot creation on the standby and the
>> cascading standby node initialization after this test?
>>
> 
> Yeah, that is also possible. But, I have a few questions regarding
> that: (a) There doesn't seem to be a physical slot on cascading
> standby, if I am missing something, can you please point me to the
> relevant part of the test?

That's right. There is a physical slot only on the primary and on the standby.

What I meant up-thread is to also postpone the cascading standby node initialization
after this test (once the physical slot on the standby is created).

Please find attached a proposal doing so.

> (b) Which test is currently dependent on
> the physical slot on standby?

Not a test but the cascading standby initialization with the "primary_slot_name" parameter.

Also, now I think that's better to have the physical slot on the standby + hsf set to on on the
cascading standby (coming from the standby backup).

Idea is to avoid any risk of logical slot invalidation on the cascading standby in the
standby promotion test.

That was not the case before the attached proposal though (hsf was off on the cascading standby).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
--------------J009g61eu3rZ9IV0rz0SMH0s
Content-Type: text/plain; charset=UTF-8;
 name="v1-0001-fix-retaining-WAL-test.patch"
Content-Disposition: attachment;
 filename="v1-0001-fix-retaining-WAL-test.patch"
Content-Transfer-Encoding: base64

RnJvbSAzOGM2N2IwZmQ4ZjJkODNlOTdhOTMxMDg0ODRmZTIzYzg4MWRkOTFjIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBCZXJ0cmFuZCBEcm91dm90IDxiZXJ0cmFuZGRyb3V2
b3QucGdAZ21haWwuY29tPgpEYXRlOiBNb24sIDggTWF5IDIwMjMgMDc6MDI6NTAgKzAwMDAK
U3ViamVjdDogW1BBVENIIHYxXSBmaXggcmV0YWluaW5nIFdBTCB0ZXN0CgotLS0KIC4uLi90
LzAzNV9zdGFuZGJ5X2xvZ2ljYWxfZGVjb2RpbmcucGwgICAgICAgICB8IDM4ICsrKysrKysr
KystLS0tLS0tLS0KIDEgZmlsZSBjaGFuZ2VkLCAyMSBpbnNlcnRpb25zKCspLCAxNyBkZWxl
dGlvbnMoLSkKIDEwMC4wJSBzcmMvdGVzdC9yZWNvdmVyeS90LwoKZGlmZiAtLWdpdCBhL3Ny
Yy90ZXN0L3JlY292ZXJ5L3QvMDM1X3N0YW5kYnlfbG9naWNhbF9kZWNvZGluZy5wbCBiL3Ny
Yy90ZXN0L3JlY292ZXJ5L3QvMDM1X3N0YW5kYnlfbG9naWNhbF9kZWNvZGluZy5wbAppbmRl
eCBhZDFkZWYyNDc0Li40YmRhNzA2ZWFlIDEwMDY0NAotLS0gYS9zcmMvdGVzdC9yZWNvdmVy
eS90LzAzNV9zdGFuZGJ5X2xvZ2ljYWxfZGVjb2RpbmcucGwKKysrIGIvc3JjL3Rlc3QvcmVj
b3ZlcnkvdC8wMzVfc3RhbmRieV9sb2dpY2FsX2RlY29kaW5nLnBsCkBAIC0yNzYsMjAgKzI3
Niw2IEBAICRub2RlX3N0YW5kYnktPmFwcGVuZF9jb25mKCdwb3N0Z3Jlc3FsLmNvbmYnLAog
ICAgICAgIG1heF9yZXBsaWNhdGlvbl9zbG90cyA9IDVdKTsKICRub2RlX3N0YW5kYnktPnN0
YXJ0OwogJG5vZGVfcHJpbWFyeS0+d2FpdF9mb3JfcmVwbGF5X2NhdGNodXAoJG5vZGVfc3Rh
bmRieSk7Ci0kbm9kZV9zdGFuZGJ5LT5zYWZlX3BzcWwoJ3Rlc3RkYicsIHFxW1NFTEVDVCAq
IEZST00gcGdfY3JlYXRlX3BoeXNpY2FsX3JlcGxpY2F0aW9uX3Nsb3QoJyRzdGFuZGJ5X3Bo
eXNpY2FsX3Nsb3RuYW1lJyk7XSk7Ci0KLSMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjCi0jIElu
aXRpYWxpemUgY2FzY2FkaW5nIHN0YW5kYnkgbm9kZQotIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMKLSRub2RlX3N0YW5kYnktPmJhY2t1cCgkYmFja3VwX25hbWUpOwotJG5vZGVfY2FzY2Fk
aW5nX3N0YW5kYnktPmluaXRfZnJvbV9iYWNrdXAoCi0JJG5vZGVfc3RhbmRieSwgJGJhY2t1
cF9uYW1lLAotCWhhc19zdHJlYW1pbmcgPT4gMSwKLQloYXNfcmVzdG9yaW5nID0+IDEpOwot
JG5vZGVfY2FzY2FkaW5nX3N0YW5kYnktPmFwcGVuZF9jb25mKCdwb3N0Z3Jlc3FsLmNvbmYn
LAotCXFxW3ByaW1hcnlfc2xvdF9uYW1lID0gJyRzdGFuZGJ5X3BoeXNpY2FsX3Nsb3RuYW1l
J10pOwotJG5vZGVfY2FzY2FkaW5nX3N0YW5kYnktPnN0YXJ0OwotJG5vZGVfc3RhbmRieS0+
d2FpdF9mb3JfcmVwbGF5X2NhdGNodXAoJG5vZGVfY2FzY2FkaW5nX3N0YW5kYnksICRub2Rl
X3ByaW1hcnkpOwogCiAjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIwogIyBJbml0aWFsaXplIHN1
YnNjcmliZXIgbm9kZQpAQCAtNTAzLDkgKzQ4OSw2IEBAIGNoZWNrX3Nsb3RzX2NvbmZsaWN0
aW5nX3N0YXR1cygxKTsKICMgVmVyaWZ5IHRoYXQgaW52YWxpZGF0ZWQgbG9naWNhbCBzbG90
cyBkbyBub3QgbGVhZCB0byByZXRhaW5pbmcgV0FMLgogIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMKIAotIyBXYWl0IGZvciB0aGUgY2FzY2Fk
aW5nIHN0YW5kYnkgdG8gY2F0Y2h1cCBiZWZvcmUgcmVtb3ZpbmcgdGhlIFdBTCBmaWxlKHMp
Ci0kbm9kZV9zdGFuZGJ5LT53YWl0X2Zvcl9yZXBsYXlfY2F0Y2h1cCgkbm9kZV9jYXNjYWRp
bmdfc3RhbmRieSwgJG5vZGVfcHJpbWFyeSk7Ci0KICMgR2V0IHRoZSByZXN0YXJ0X2xzbiBm
cm9tIGFuIGludmFsaWRhdGVkIHNsb3QKIG15ICRyZXN0YXJ0X2xzbiA9ICRub2RlX3N0YW5k
YnktPnNhZmVfcHNxbCgncG9zdGdyZXMnLAogCSJTRUxFQ1QgcmVzdGFydF9sc24gZnJvbSBw
Z19yZXBsaWNhdGlvbl9zbG90cyBXSEVSRSBzbG90X25hbWUgPSAndmFjdXVtX2Z1bGxfYWN0
aXZlc2xvdCcgYW5kIGNvbmZsaWN0aW5nIGlzIHRydWU7IgpAQCAtNTM4LDYgKzUyMSwyNyBA
QCBteSAkc3RhbmRieV93YWxmaWxlID0gJG5vZGVfc3RhbmRieS0+ZGF0YV9kaXIgLiAnL3Bn
X3dhbC8nIC4gJHdhbGZpbGVfbmFtZTsKIG9rKCEtZiAiJHN0YW5kYnlfd2FsZmlsZSIsCiAJ
ImludmFsaWRhdGVkIGxvZ2ljYWwgc2xvdHMgZG8gbm90IGxlYWQgdG8gcmV0YWluaW5nIFdB
TCIpOwogCisjIE5vdyBjcmVhdGUgdGhlIHBoeXNpY2FsIHNsb3Qgb24gdGhlIHN0YW5kYnkg
YW5kIGluaXRpYWxpemUgdGhlIGNhc2NhZGluZyBzdGFuZGJ5IG5vZGUuCisjIEl0J3MgZG9u
ZSBoZXJlIGJlY2F1c2UgdGhlIHBoeXNpY2FsIHNsb3Qgb24gdGhlIHN0YW5kYnkgY291bGQg
cHJldmVudCB0aGUgV0FMCisjIGZpbGUgdG8gYmUgcmVtb3ZlZCBpbiB0aGUgdGVzdCBhYm92
ZSAoZW5zdXJpbmcgdGhlIFdBTCBmaWxlIHJlbW92YWwgY291bGQgc3RpbGwgYmUKKyMgcG9z
c2libGUgYnV0IHdlIHdhbnQgdG8ga2VlcCB0aGUgYWJvdmUgdGVzdCBzaW1wbGUpLgorIyBB
bHNvIHBsZWFzZSBub3RlIHRoYXQgdGhlIGNhc2NhZGluZyBzdGFuZGJ5IHdpbGwgYmUgY3Jl
YXRlZCB3aXRoIGhvdF9zdGFuZGJ5X2ZlZWRiYWNrCisjIHNldCB0byBvbiB0aGFua3MgdG8g
dGhlIGxhc3QgY2hhbmdlX2hvdF9zdGFuZGJ5X2ZlZWRiYWNrX2FuZF93YWl0X2Zvcl94bWlu
cygpIGNhbGwuCiskbm9kZV9zdGFuZGJ5LT5zYWZlX3BzcWwoJ3Rlc3RkYicsIHFxW1NFTEVD
VCAqIEZST00gcGdfY3JlYXRlX3BoeXNpY2FsX3JlcGxpY2F0aW9uX3Nsb3QoJyRzdGFuZGJ5
X3BoeXNpY2FsX3Nsb3RuYW1lJyk7XSk7CisKKyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjCisj
IEluaXRpYWxpemUgY2FzY2FkaW5nIHN0YW5kYnkgbm9kZQorIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMKKyRub2RlX3N0YW5kYnktPmJhY2t1cCgkYmFja3VwX25hbWUpOworJG5vZGVfY2Fz
Y2FkaW5nX3N0YW5kYnktPmluaXRfZnJvbV9iYWNrdXAoCisJJG5vZGVfc3RhbmRieSwgJGJh
Y2t1cF9uYW1lLAorCWhhc19zdHJlYW1pbmcgPT4gMSwKKwloYXNfcmVzdG9yaW5nID0+IDEp
OworJG5vZGVfY2FzY2FkaW5nX3N0YW5kYnktPmFwcGVuZF9jb25mKCdwb3N0Z3Jlc3FsLmNv
bmYnLAorCXFxW3ByaW1hcnlfc2xvdF9uYW1lID0gJyRzdGFuZGJ5X3BoeXNpY2FsX3Nsb3Ru
YW1lJ10pOworJG5vZGVfY2FzY2FkaW5nX3N0YW5kYnktPnN0YXJ0OworJG5vZGVfc3RhbmRi
eS0+d2FpdF9mb3JfcmVwbGF5X2NhdGNodXAoJG5vZGVfY2FzY2FkaW5nX3N0YW5kYnksICRu
b2RlX3ByaW1hcnkpOworCiAjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIwogIyBSZWNvdmVyeSBjb25mbGljdDogSW52YWxpZGF0ZSBjb25mbGlj
dGluZyBzbG90cywgaW5jbHVkaW5nIGluLXVzZSBzbG90cwogIyBTY2VuYXJpbyAyOiBjb25m
bGljdCBkdWUgdG8gcm93IHJlbW92YWwgd2l0aCBob3Rfc3RhbmRieV9mZWVkYmFjayBvZmYu
Ci0tIAoyLjM0LjEKCg==

--------------J009g61eu3rZ9IV0rz0SMH0s--