pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Fix the intermittent buildfarm failures in 031_column_list.
4+ messages / 3 participants
[nested] [flat]

* pgsql: Fix the intermittent buildfarm failures in 031_column_list.
@ 2024-02-22 06:48  Amit Kapila <akapila@postgresql.org>
  0 siblings, 0 replies; 4+ messages in thread

From: Amit Kapila @ 2024-02-22 06:48 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix the intermittent buildfarm failures in 031_column_list.

The reason was that the ALTER SUBSCRIPTION .. SET PUBLICATION will lead to
the restarting of apply worker and after the restart, the apply worker
will use the existing slot and replication origin corresponding to the
subscription. Now, it is possible that before restart the origin has not
been updated and the WAL start location points to a location before where
PUBLICATION exists which can lead to the error "publication ... does not
exist".

Fix it by recreating the subscription as a newly created subscription will
start processing WAL from the recent WAL location and will see the
required publication.

This behavior has existed from the time logical replication was introduced
but is exposed by this test and we have started a discussion for a better
fix for this problem.

As per Buildfarm

Diagnosed-by: Amit Kapila
Author: Vignesh C
Discussion: https://postgr.es/m/3307255.1706911634@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/b6df0798a5e2480d28cceee7df3f58650cf6fa7a

Modified Files
--------------
src/test/subscription/t/031_column_list.pl | 43 +++++++++++++++++++-----------
1 file changed, 28 insertions(+), 15 deletions(-)



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* pgsql: Fix the intermittent buildfarm failures in 040_standby_failover_
@ 2024-04-08 08:03  Amit Kapila <akapila@postgresql.org>
  0 siblings, 1 reply; 4+ messages in thread

From: Amit Kapila @ 2024-04-08 08:03 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix the intermittent buildfarm failures in 040_standby_failover_slots_sync.

It is possible that even if the primary waits for the subscriber to catch
up and then disables the subscription, the XLOG_RUNNING_XACTS record gets
inserted between the two steps by bgwriter and walsender processes it.
This can move the restart_lsn of the corresponding slot in an
unpredictable way which further leads to slot sync failure.

To ensure predictable behaviour, we drop the subscription and manually
create the slot before the test. The other idea we discussed to write a
predictable test is to use injection points to control the bgwriter
logging XLOG_RUNNING_XACTS but that needs more analysis. We can add a
separate test using injection points.

Per buildfarm

Author: Hou Zhijie
Reviewed-by: Amit Kapila, Shveta Malik
Discussion: https://postgr.es/m/CAA4eK1JD8h_XLRsK_o_Xh=5MhTzm+6d4Cb4_uPgFJ2wSQDah=g@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/6f3d8d5e7cc2f2e2367cd6da6f8affe98d1f5729

Modified Files
--------------
.../recovery/t/040_standby_failover_slots_sync.pl  | 62 +++++++++-------------
1 file changed, 24 insertions(+), 38 deletions(-)



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: pgsql: Fix the intermittent buildfarm failures in 040_standby_failover_
@ 2024-04-08 15:53  Robert Haas <robertmhaas@gmail.com>
  parent: Amit Kapila <akapila@postgresql.org>
  0 siblings, 1 reply; 4+ messages in thread

From: Robert Haas @ 2024-04-08 15:53 UTC (permalink / raw)
  To: Amit Kapila <akapila@postgresql.org>; +Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>

On Mon, Apr 8, 2024 at 4:04 AM Amit Kapila <akapila@postgresql.org> wrote:
> Fix the intermittent buildfarm failures in 040_standby_failover_slots_sync.
>
> It is possible that even if the primary waits for the subscriber to catch
> up and then disables the subscription, the XLOG_RUNNING_XACTS record gets
> inserted between the two steps by bgwriter and walsender processes it.
> This can move the restart_lsn of the corresponding slot in an
> unpredictable way which further leads to slot sync failure.
>
> To ensure predictable behaviour, we drop the subscription and manually
> create the slot before the test. The other idea we discussed to write a
> predictable test is to use injection points to control the bgwriter
> logging XLOG_RUNNING_XACTS but that needs more analysis. We can add a
> separate test using injection points.

Hi,

I'm concerned that the failover slots feature may not be in
sufficiently good shape for us to ship it. Since this test file was
introduced at the end of January, it's been touched by a total of 16
commits, most of which seem to be trying to get it to pass reliably:

6f3d8d5e7c Fix the intermittent buildfarm failures in
040_standby_failover_slots_sync.
6f132ed693 Allow synced slots to have their inactive_since.
2ec005b4e2 Ensure that the sync slots reach a consistent state after
promotion without losing data.
6ae701b437 Track invalidation_reason in pg_replication_slots.
bf279ddd1c Introduce a new GUC 'standby_slot_names'.
def0ce3370 Fix BF failure introduced by commit b3f6b14cf4.
b3f6b14cf4 Fixups for commit 93db6cbda0.
d13ff82319 Fix BF failure in commit 93db6cbda0.
93db6cbda0 Add a new slot sync worker to synchronize logical slots.
801792e528 Improve ERROR/LOG messages added by commits ddd5f4f54a and
7a424ece48.
b7bdade6a4 Disable autovacuum on primary in
040_standby_failover_slots_sync test.
d9e225f275 Change the LOG level in 040_standby_failover_slots_sync.pl to DEBUG2.
9bc1eee988 Another try to fix BF failure introduced in commit ddd5f4f54a.
bd8fc1677b Fix BF introduced in commit ddd5f4f54a.
ddd5f4f54a Add a slot synchronization function.
776621a5e4 Add a failover option to subscriptions.

It's not really the test failures themselves that concern me here, so
much as the possibility of users who try to make use of this feature
having a similar amount of difficulty getting it to work reliably. The
test case seems to be taking more and more elaborate precautions to
prevent incidental things from breaking the feature. But users won't
like this feature very much if they have to take elaborate precautions
to get it to work in the real world. Is there a reason to believe that
all of this stabilization work is addressing artificial problems that
won't inconvenience real users, or should we be concerned that the
feature itself is going to be difficult to use effectively?

-- 
Robert Haas
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: pgsql: Fix the intermittent buildfarm failures in 040_standby_failover_
@ 2024-04-09 02:07  Amit Kapila <amit.kapila16@gmail.com>
  parent: Robert Haas <robertmhaas@gmail.com>
  0 siblings, 0 replies; 4+ messages in thread

From: Amit Kapila @ 2024-04-09 02:07 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; +Cc: Amit Kapila <akapila@postgresql.org>; pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>

On Mon, Apr 8, 2024 at 9:24 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> Hi,
>
> I'm concerned that the failover slots feature may not be in
> sufficiently good shape for us to ship it. Since this test file was
> introduced at the end of January, it's been touched by a total of 16
> commits, most of which seem to be trying to get it to pass reliably:
>

Among the 16 commits, there are 6 feature commits (3 of which are
another feature that has interaction with this feature), 1 code
improvement commit, 2 bug fixes commit, and 7 test stabilization
commits. See [1] for the categorization of commits. Now, among these 7
test stabilization commits (which seems to be the main source of your
concern), 4 are due to the reason that we are expecting the slots to
be synced in one function call with pg_sync_replication_slots() which
sometimes didn't happen when there is an unexpected WAL generation say
by bgwriter (XLOG_RUNNING_XACTS ) or an extra XID generation by
auto(analyze). This shouldn't be a problem in practice where users are
expected to use slotsync worker which will keep syncing slots at
regular intervals. All these required stabilizations are in two of the
tests involving the use of the function pg_sync_replication_slots() to
sync slots. We can think of getting rid of this function and relying
only on slotsync worker functionality but I find this function quite
convenient for debugging and in some cases writing targeted tests
(though it caused instability in tests). We can provide more
information in docs for the use of this API.

The other stabilization fixes are as follows: 1 is a Perl scripting
issue to check LOGs, 1 is to increase the DEBUG level to catch more
information for failures, and 1 is a test setup miss which is already
done in other similar tests.

Having said that, I have kept an eye on the reports (-hackers, -bugs,
etc.) related to this feature and if we find that this feature is
inconvenient to use then we should consider either improving it, if
possible, or reverting it.

[1]:
New features:
6f132ed693 Allow synced slots to have their inactive_since.
6ae701b437 Track invalidation_reason in pg_replication_slots.
bf279ddd1c Introduce a new GUC 'standby_slot_names'.
93db6cbda0 Add a new slot sync worker to synchronize logical slots.
ddd5f4f54a Add a slot synchronization function.
776621a5e4 Add a failover option to subscriptions.

Code improvement
801792e528 Improve ERROR/LOG messages added by commits ddd5f4f54a and
7a424ece48.

Bug fixes:
2ec005b4e2 Ensure that the sync slots reach a consistent state after
promotion without losing data.
b3f6b14cf4 Fixups for commit 93db6cbda0.

Stabilize test cases:
def0ce3370 Fix BF failure introduced by commit b3f6b14cf4.
b7bdade6a4 Disable autovacuum on primary in
040_standby_failover_slots_sync test.
d9e225f275 Change the LOG level in 040_standby_failover_slots_sync.pl to DEBUG2.
9bc1eee988 Another try to fix BF failure introduced in commit ddd5f4f54a.
bd8fc1677b Fix BF introduced in commit ddd5f4f54a.
d13ff82319 Fix BF failure in commit 93db6cbda0.
6f3d8d5e7c Fix the intermittent buildfarm failures in
040_standby_failover_slots_sync.

-- 
With Regards,
Amit Kapila.





^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2024-04-09 02:07 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-02-22 06:48 pgsql: Fix the intermittent buildfarm failures in 031_column_list. Amit Kapila <akapila@postgresql.org>
2024-04-08 08:03 pgsql: Fix the intermittent buildfarm failures in 040_standby_failover_ Amit Kapila <akapila@postgresql.org>
2024-04-08 15:53 ` Re: pgsql: Fix the intermittent buildfarm failures in 040_standby_failover_ Robert Haas <robertmhaas@gmail.com>
2024-04-09 02:07   ` Re: pgsql: Fix the intermittent buildfarm failures in 040_standby_failover_ Amit Kapila <amit.kapila16@gmail.com>

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