On Tue, Aug 8, 2023 at 11:08 AM Andres Freund <and...@anarazel.de> wrote: > On 2023-08-07 12:57:40 +0200, Christoph Berg wrote: > > v8 worked better. It succeeded a few times (at least 12, my screen > > scrollback didn't catch more) before erroring like this: > > > [10:21:58.410](0.151s) ok 15 - startup deadlock: logfile contains > > terminated connection due to recovery conflict > > [10:21:58.463](0.053s) not ok 16 - startup deadlock: stats show conflict on > > standby > > [10:21:58.463](0.000s) > > [10:21:58.463](0.000s) # Failed test 'startup deadlock: stats show > > conflict on standby' > > # at t/031_recovery_conflict.pl line 332. > > [10:21:58.463](0.000s) # got: '0' > > # expected: '1' > > Hm, that could just be a "harmless" race. Does it still happen if you apply > the attached patch in addition?
Do you intend that as the fix, or just proof of what's wrong? It looks pretty reasonable to me. I will go ahead and commit this unless you want to change something/do it yourself. As far as I know so far, this is the last thing preventing the mighty Debian/s390x build box from passing consistently, which would be nice.
From cbab5797fde1db70464f157b4bb9321ab40ad025 Mon Sep 17 00:00:00 2001 From: Thomas Munro <tmu...@postgresql.org> Date: Thu, 7 Sep 2023 15:28:47 +1200 Subject: [PATCH] Update recovery conflict stats immediately. 031_recovery_conflict.pl would occasionally fail to see a stats change, leading to intermittent failures. Since recovery conflicts are rare, just flush them immediately. No back-patch for now, but that will be needed if/when the test is re-enabled in back-branches. Author: Andres Freund <and...@anarazel.de> Reported-by: Christoph Berg <m...@debian.org> Discussion: https://postgr.es/m/20230807230821.gylo74fox445hu24%40awork3.anarazel.de diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c index d04426f53f..cc2a5724e7 100644 --- a/src/backend/utils/activity/pgstat_database.c +++ b/src/backend/utils/activity/pgstat_database.c @@ -81,12 +81,22 @@ void pgstat_report_recovery_conflict(int reason) { PgStat_StatDBEntry *dbentry; + PgStat_EntryRef *entry_ref; + PgStatShared_Database *sharedent; Assert(IsUnderPostmaster); if (!pgstat_track_counts) return; - dbentry = pgstat_prep_database_pending(MyDatabaseId); + /* + * Update the shared stats directly - recovery conflicts should never be + * common enough for that to be a problem. + */ + entry_ref = + pgstat_get_entry_ref_locked(PGSTAT_KIND_DATABASE, MyDatabaseId, InvalidOid, false); + + sharedent = (PgStatShared_Database *) entry_ref->shared_stats; + dbentry = &sharedent->stats; switch (reason) { @@ -116,6 +126,8 @@ pgstat_report_recovery_conflict(int reason) dbentry->conflict_startup_deadlock++; break; } + + pgstat_unlock_entry(entry_ref); } /* -- 2.41.0
