Content-Type: multipart/mixed; boundary="------------iybOSqol8700jeuLMbxLtLYk"
Message-ID: <054defef-de15-53e4-6c5a-d16ccc21d4ba@gmail.com>
Date: Fri, 28 Apr 2023 10:54:00 +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: "Yu Shi (Fujitsu)" <shiy.fnst@fujitsu.com>,
 vignesh C <vignesh21@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <2fefa454-5a70-2174-ddbf-4a0e41537139@gmail.com>
 <CALDaNm2raNJrEF_ucDeTV_AygYutVb1bz2SrKO06YdVVG1gf2A@mail.gmail.com>
 <9ab1827c-9a57-8c48-59b2-c5fb62c21be3@gmail.com>
 <CAA4eK1KVoAjzT49BwKCih0A+1PE7hm=4PwQ-_VUr9YteFJRB5g@mail.gmail.com>
 <CAA4eK1JeAZ9MUf3qPAOA5t9=e_vPJSEi6nZ3OygDYi-913eO+Q@mail.gmail.com>
 <74f80bc1-6b39-b2a8-c716-98beefb75e0b@gmail.com>
 <OSZPR01MB6310F24417BDD04731BD7978FD659@OSZPR01MB6310.jpnprd01.prod.outlook.com>
 <17b6b056-4f53-c444-0c94-869547d1a5df@gmail.com>
 <CAA4eK1JiYisk1aqn1uvjd+VCfsVtXLby7uMmkLQU_ggAx7xt6w@mail.gmail.com>
Content-Language: en-US
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
In-Reply-To: 
 <CAA4eK1JiYisk1aqn1uvjd+VCfsVtXLby7uMmkLQU_ggAx7xt6w@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/054defef-de15-53e4-6c5a-d16ccc21d4ba%40gmail.com>
Precedence: bulk

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

Hi,

On 4/28/23 5:55 AM, Amit Kapila wrote:
> On Wed, Apr 26, 2023 at 7:53 PM Drouvot, Bertrand
> <bertranddrouvot.pg@gmail.com> wrote:
> 
> +# Get the restart_lsn from an invalidated slot
> +my $restart_lsn = $node_standby->safe_psql('postgres',
> + "SELECT restart_lsn from pg_replication_slots WHERE slot_name =
> 'vacuum_full_activeslot' and conflicting is true;"
> +);
> +
> +chomp($restart_lsn);
> +
> +# Get the WAL file name associated to this lsn on the primary
> +my $walfile_name = $node_primary->safe_psql('postgres',
> + "SELECT pg_walfile_name('$restart_lsn')");
> +
> +chomp($walfile_name);
> +
> +# Check the WAL file is still on the primary
> +ok(-f $node_primary->data_dir . '/pg_wal/' . $walfile_name,
> + "WAL file still on the primary");
> 
> How is it guaranteed that the WAL file corresponding to the
> invalidated slot on standby will still be present on primary?

The slot(s) have been invalidated by the "vacuum full" test just above
this one. So I think the WAL we are looking for is the last one being used
by the primary. As no activity happened on it since the vacuum full it looks to
me that it should still be present.

But I may have missed something and maybe that's not guarantee that this WAL is still there in all the cases.
In that case I think it's better to remove this test (it does not provide added value here).

Test removed in V7 attached.

> Can you
> please explain the logic behind this test a bit more like how the WAL
> file switch helps you to achieve the purpose?
> 

The idea was to generate enough "wal switch" on the primary to ensure
the WAL file has been removed.

I gave another thought on it and I think we can skip the test that the WAL is
not on the primary any more. That way, one "wal switch" seems to be enough
to see it removed on the standby.

It's done in V7.

V7 is not doing "extra tests" than necessary and I think it's probably better like this.

I can see V7 failing on "Cirrus CI / macOS - Ventura - Meson" only (other machines are not complaining).

It does fail on "invalidated logical slots do not lead to retaining WAL", see https://cirrus-ci.com/task/4518083541336064

I'm not sure why it is failing, any idea?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
--------------iybOSqol8700jeuLMbxLtLYk
Content-Type: text/plain; charset=UTF-8;
 name="v7-0001-Add-a-test-to-verify-that-invalidated-logical-slo.patch"
Content-Disposition: attachment;
 filename*0="v7-0001-Add-a-test-to-verify-that-invalidated-logical-slo.pa";
 filename*1="tch"
Content-Transfer-Encoding: base64

RnJvbSAyYWIwODIxNDQxNTAyMzUwNTI0NGM5NTRhNmE1ZWJmNDJlYzlhZWJiIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBCZXJ0cmFuZCBEcm91dm90IDxiZXJ0cmFuZGRyb3V2
b3QucGdAZ21haWwuY29tPgpEYXRlOiBGcmksIDI4IEFwciAyMDIzIDA3OjI3OjIwICswMDAw
ClN1YmplY3Q6IFtQQVRDSCB2N10gQWRkIGEgdGVzdCB0byB2ZXJpZnkgdGhhdCBpbnZhbGlk
YXRlZCBsb2dpY2FsIHNsb3RzIGRvIG5vdAogbGVhZCB0byByZXRhaW5pbmcgV0FMLgoKLS0t
CiAuLi4vdC8wMzVfc3RhbmRieV9sb2dpY2FsX2RlY29kaW5nLnBsICAgICAgICAgfCAzOSAr
KysrKysrKysrKysrKysrKystCiAxIGZpbGUgY2hhbmdlZCwgMzcgaW5zZXJ0aW9ucygrKSwg
MiBkZWxldGlvbnMoLSkKIDEwMC4wJSBzcmMvdGVzdC9yZWNvdmVyeS90LwoKZGlmZiAtLWdp
dCBhL3NyYy90ZXN0L3JlY292ZXJ5L3QvMDM1X3N0YW5kYnlfbG9naWNhbF9kZWNvZGluZy5w
bCBiL3NyYy90ZXN0L3JlY292ZXJ5L3QvMDM1X3N0YW5kYnlfbG9naWNhbF9kZWNvZGluZy5w
bAppbmRleCA2NmQyNjRmMjMwLi5iMzJjMTAwMmIwIDEwMDY0NAotLS0gYS9zcmMvdGVzdC9y
ZWNvdmVyeS90LzAzNV9zdGFuZGJ5X2xvZ2ljYWxfZGVjb2RpbmcucGwKKysrIGIvc3JjL3Rl
c3QvcmVjb3ZlcnkvdC8wMzVfc3RhbmRieV9sb2dpY2FsX2RlY29kaW5nLnBsCkBAIC01MDAs
OSArNTAwLDQ0IEBAICRub2RlX3N0YW5kYnktPnJlc3RhcnQ7CiBjaGVja19zbG90c19jb25m
bGljdGluZ19zdGF0dXMoMSk7CiAKICMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMjIyMjCi0jIFZlcmlmeSB0aGF0IGludmFsaWRhdGVkIGxvZ2ljYWwg
c2xvdHMgZG8gbm90IGxlYWQgdG8gcmV0YWluaW5nIFdBTAorIyBWZXJpZnkgdGhhdCBpbnZh
bGlkYXRlZCBsb2dpY2FsIHNsb3RzIGRvIG5vdCBsZWFkIHRvIHJldGFpbmluZyBXQUwuCiAj
IyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIwotIyBY
WFhYWCBUT0RPCisKKyMgQmVmb3JlIHJlbW92aW5nIFdBTCBmaWxlKHMpLCBlbnN1cmUgdGhl
IGNhc2NhZGluZyBzdGFuZGJ5IGNhdGNoIHVwCiskbm9kZV9zdGFuZGJ5LT53YWl0X2Zvcl9y
ZXBsYXlfY2F0Y2h1cCgkbm9kZV9jYXNjYWRpbmdfc3RhbmRieSwKKwkkbm9kZV9wcmltYXJ5
KTsKKworIyBHZXQgdGhlIHJlc3RhcnRfbHNuIGZyb20gYW4gaW52YWxpZGF0ZWQgc2xvdAor
bXkgJHJlc3RhcnRfbHNuID0gJG5vZGVfc3RhbmRieS0+c2FmZV9wc3FsKCdwb3N0Z3Jlcycs
CisJIlNFTEVDVCByZXN0YXJ0X2xzbiBmcm9tIHBnX3JlcGxpY2F0aW9uX3Nsb3RzIFdIRVJF
IHNsb3RfbmFtZSA9ICd2YWN1dW1fZnVsbF9hY3RpdmVzbG90JyBhbmQgY29uZmxpY3Rpbmcg
aXMgdHJ1ZTsiCispOworCitjaG9tcCgkcmVzdGFydF9sc24pOworCisjIEFzIHBnX3dhbGZp
bGVfbmFtZSgpIGNhbiBub3QgYmUgZXhlY3V0ZWQgb24gdGhlIHN0YW5kYnksCisjIGdldCB0
aGUgV0FMIGZpbGUgbmFtZSBhc3NvY2lhdGVkIHRvIHRoaXMgbHNuIGZyb20gdGhlIHByaW1h
cnkKK215ICR3YWxmaWxlX25hbWUgPSAkbm9kZV9wcmltYXJ5LT5zYWZlX3BzcWwoJ3Bvc3Rn
cmVzJywKKwkiU0VMRUNUIHBnX3dhbGZpbGVfbmFtZSgnJHJlc3RhcnRfbHNuJykiKTsKKwor
Y2hvbXAoJHdhbGZpbGVfbmFtZSk7CisKKyMgR2VuZXJhdGUgc29tZSBhY3Rpdml0eSBhbmQg
c3dpdGNoIFdBTCBmaWxlIG9uIHRoZSBwcmltYXJ5Ciskbm9kZV9wcmltYXJ5LT5zYWZlX3Bz
cWwoCisJJ3Bvc3RncmVzJywgImNyZWF0ZSB0YWJsZSByZXRhaW5fdGVzdChhIGludCk7CisJ
CQkJCQkJCQkgaW5zZXJ0IGludG8gcmV0YWluX3Rlc3QgdmFsdWVzKDEpOworCQkJCQkJCQkJ
IHNlbGVjdCBwZ19zd2l0Y2hfd2FsKCk7CisJCQkJCQkJCQkgY2hlY2twb2ludDsiKTsKKwor
IyBXYWl0IGZvciB0aGUgc3RhbmRieSB0byBjYXRjaCB1cAorJG5vZGVfcHJpbWFyeS0+d2Fp
dF9mb3JfY2F0Y2h1cCgkbm9kZV9zdGFuZGJ5KTsKKworIyBSZXF1ZXN0IGEgY2hlY2twb2lu
dCBvbiB0aGUgc3RhbmRieSB0byB0cmlnZ2VyIHRoZSBXQUwgZmlsZShzKSByZW1vdmFsCisk
bm9kZV9zdGFuZGJ5LT5zYWZlX3BzcWwoJ3Bvc3RncmVzJywgJ2NoZWNrcG9pbnQ7Jyk7CisK
KyMgVmVyaWZ5IHRoYXQgdGhlIHdhbCBmaWxlIGhhcyBub3QgYmVlbiByZXRhaW5lZCBvbiB0
aGUgc3RhbmRieQorbXkgJHN0YW5kYnlfd2FsZmlsZSA9ICRub2RlX3N0YW5kYnktPmRhdGFf
ZGlyIC4gJy9wZ193YWwvJyAuICR3YWxmaWxlX25hbWU7CitvayghLWYgIiRzdGFuZGJ5X3dh
bGZpbGUiLAorCSJpbnZhbGlkYXRlZCBsb2dpY2FsIHNsb3RzIGRvIG5vdCBsZWFkIHRvIHJl
dGFpbmluZyBXQUwiKTsKIAogIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMKICMgUmVjb3ZlcnkgY29uZmxpY3Q6IEludmFsaWRhdGUgY29uZmxp
Y3Rpbmcgc2xvdHMsIGluY2x1ZGluZyBpbi11c2Ugc2xvdHMKLS0gCjIuMzQuMQoK

--------------iybOSqol8700jeuLMbxLtLYk--