From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Yu Shi (Fujitsu) <shiy.fnst@fujitsu.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Test slots invalidations in 035_standby_logical_decoding.pl only if dead rows are removed
Date: Fri, 19 Jan 2024 09:00:01 +0300
Message-ID: <d975bb14-2c90-25bf-7d71-7ce5f9d7fa82@gmail.com> (raw)
In-Reply-To: <b641be2a-945b-999c-2828-7bbd8a16a009@gmail.com>
References: <ZZ4cZkBLczQISHxq@paquier.xyz>
<7c025095-5763-17a6-8c80-899b35bd0459@gmail.com>
<ZZ7GpII4bAYN+jT5@ip-10-97-1-34.eu-west-3.compute.internal>
<5d0e5bec-67f9-9164-36cb-c4ff5f95d1ed@gmail.com>
<ZaACEPtVovKlRVUf@ip-10-97-1-34.eu-west-3.compute.internal>
<6f85667e-5754-5d35-dbf1-c83fe08c1e48@gmail.com>
<ZaBlU5FqOeDqyHtJ@paquier.xyz>
<ZaDnIMTbT+0yR5LZ@ip-10-97-1-34.eu-west-3.compute.internal>
<cc7925b8-30cc-c76d-b1b6-c9ec6bd36a03@gmail.com>
<ZaFCoJ1FxBQ9jQkX@ip-10-97-1-34.eu-west-3.compute.internal>
<ZaSthTtVmvKKuGNb@paquier.xyz>
<b641be2a-945b-999c-2828-7bbd8a16a009@gmail.com>
Hello,
15.01.2024 12:00, Alexander Lakhin wrote:
> If this approach looks promising to you, maybe we could add a submodule to
> perl/PostgreSQL/Test/ and use this functionality in other tests (e.g., in
> 019_replslot_limit) as well.
>
> Personally I think that having such a functionality for using in tests
> might be useful not only to avoid some "problematic" behaviour but also to
> test the opposite cases.
After spending a few days on it, I've discovered two more issues:
https://www.postgresql.org/message-id/16d6d9cc-f97d-0b34-be65-425183ed3721%40gmail.comhttps://www.postgresql.org/message-id/b0102688-6d6c-c86a-db79-e0e91d245b1a%40gmail.com
(The latter is not related to bgwriter directly, but it was discovered
thanks to the RUNNING_XACTS record flew in WAL in a lucky moment.)
So it becomes clear that the 035 test is not the only one, which might
suffer from bgwriter's activity, and inventing a way to stop bgwriter/
control it is a different subject, getting out of scope of the current
issue.
15.01.2024 11:49, Bertrand Drouvot wrote:
> We did a few things in this thread, so to sum up what we've discovered:
>
> - a race condition in InvalidatePossiblyObsoleteSlot() (see [1])
> - we need to launch the vacuum(s) only if we are sure we got a newer XID horizon
> ( proposal in in v6 attached)
> - we need a way to control how frequent xl_running_xacts are emmitted (to ensure
> they are not triggered in a middle of an active slot invalidation test).
>
> I'm not sure it's possible to address Tom's concern and keep the test "predictable".
>
> So, I think I'd vote for Michael's proposal to implement a superuser-settable
> developer GUC (as sending a SIGSTOP on the bgwriter (and bypass $windows_os) would
> still not address Tom's concern anyway).
>
> Another option would be to "sacrifice" the full predictablity of the test (in
> favor of real-world behavior testing)?
>
> [1]: https://www.postgresql.org/message-id/ZaTjW2Xh%2BTQUCOH0%40ip-10-97-1-34.eu-west-3.compute.internal
So, now we have the test 035 failing due to nondeterministic vacuum
activity in the first place, and due to bgwriter's activity in the second.
Maybe it would be a right move to commit the fix, and then think about
more rare failures.
Though I have a couple of question regarding the fix left, if you don't
mind:
1) The test has minor defects in the comments, that I noted before [1];
would you like to fix them in passing?
> BTW, it looks like the comment:
> # One way to produce recovery conflict is to create/drop a relation and
> # launch a vacuum on pg_class with hot_standby_feedback turned off on the standby.
> in scenario 3 is a copy-paste error.
> Also, there are two "Scenario 4" in this test.
>
2) Shall we align the 035_standby_logical_decoding with
031_recovery_conflict in regard to improving stability of vacuum?
I see the following options for this:
a) use wait_until_vacuum_can_remove() and autovacuum = off in both tests;
b) use FREEZE and autovacuum = off in both tests;
c) use wait_until_vacuum_can_remove() in 035, FREEZE in 031, and
autovacuum = off in both.
I've re-tested the v6 patch today and confirmed that it makes the test
more stable. I ran (with bgwriter_delay = 10000 in temp.config) 20 tests in
parallel and got failures ('inactiveslot slot invalidation is logged with
vacuum on pg_authid') on iterations 2, 6, 6 with no patch applied.
(With unlimited CPU, when average test duration is around 70 seconds.)
But with v6 applied, 60 iterations succeeded.
[1] https://www.postgresql.org/message-id/b2a1f7d0-7629-72c0-2da7-e9c4e336b010@gmail.com
Best regards,
Alexander
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: exclusion@gmail.com, michael@paquier.xyz, bertranddrouvot.pg@gmail.com, shiy.fnst@fujitsu.com, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, andres@anarazel.de, thomas.munro@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Test slots invalidations in 035_standby_logical_decoding.pl only if dead rows are removed
In-Reply-To: <d975bb14-2c90-25bf-7d71-7ce5f9d7fa82@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox