Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 16 Jun 2023 10:01:33 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: will@extrahop.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17973: Reinit of pgstats entry for dropped DB can break
 autovacuum daemon
Message-ID: <ZIu0bR6qSArdm6bw@paquier.xyz>
References: <17973-bca1f7d5c14f601e@postgresql.org>
 <20230614213724.gweo666oxqrugm44@awork3.anarazel.de>
 <20230614215744.ddeuii7jcojbp3yv@awork3.anarazel.de>
 <20230614223012.cfyzxyfnt3ggqkt4@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="6qU/fYYiy7OeI3nj"
Content-Disposition: inline
In-Reply-To: <20230614223012.cfyzxyfnt3ggqkt4@awork3.anarazel.de>
Archived-At: 
 <https://www.postgresql.org/message-id/ZIu0bR6qSArdm6bw%40paquier.xyz>
Precedence: bulk


--6qU/fYYiy7OeI3nj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 14, 2023 at 03:30:12PM -0700, Andres Freund wrote:
> Attached is a rough prototype implementing this idea.  Could you check if=
 that
> fixes the issue?

It requires a few manual steps, but I have been able to stuck the
autovacuum launcher schedule.  Nice investigation from the reporters.

I may be missing something here, but finishing with an inconsistent
database list (generated based on the pgstat database entries) in the
autovacuum launcher is not something that can happen only because of a
worker, right?  A normal backend would call pgstat_update_dbstats()
once it exists, re-creating a fresh entry with the dropped database
OID.  Is that right?

+   /*
+    * If we haven't connected to a database yet, don't attribute time to
+    * "shared state" (InvalidOid is used to track stats for shared relatio=
ns
+    * etc).
+    */
+   if (!OidIsValid(MyDatabaseId))
+       return;

Hmm.  pgstat_report_stat() is called by standby_redo() for a
XLOG_RUNNING_XACTS record so this would prevent the startup process
=66rom doing any stats updates for the shared db state, no?

Looking at the most recent things in this area, like bc49d93 or
ac23b71, I don't immediately see why your new ordering suggestion
would not be OK with MyDatabaseId getting set before acquiring the
shared lock of the database, so as the stat entries don't get messed
up.  The result feels cleaner in the initialization sequence,
additionally.  So, nice.
--
Michael

--6qU/fYYiy7OeI3nj
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmSLtG0ACgkQnvQgOdby
QH0LHRAAgnRqWGtRJCI3rm0ewS/tNVc7y0Q63hvEh7v+zJ6WrgjQQI5rjOhIKTIC
7xa4nTw/3cAL80iu5MrCViU6Ymitv/gtMnKqK7zST37x1QflGXaAW3TLE5uWiF2X
V4+oTrm4nKNNFahSY3+qAhcIMfNvHQrSC55hSak/bD7om+oQj8v0maRkXuo2IDc8
F2XDQ2w3AMe4KZb8pJgqgHjUL8NacL8sop079fY8Hk0yuURe+4TF612CZu0KMr0E
24KWWSJu44GbAsHU13XAeqmySFZ9v6p5FuRN3RzKHP9Q/R6cYczOjGTRPRsyvBfg
2AiUz9e7Md+Yh0KkxNmBMd6D58utoNza3gniK2Rb2R8r5Wi7AdsfH5uXMqEYmA4G
Ya0MzIjsKiX6/KPoLieItyo1sGX+jYvs6MJX59HiVY/Tzqie/FZZKfmQHy2hHLhX
5/2GpYcv6f2s7Yzp/7VkX7EmlPFo+mNydLwfkN/YiO7v5/U9MJW2od+Mwt/f90G4
eJOeMIaXohA9EsvqUFAKl8xlCVIXmAthnOj4yYF/v1Dm9vaXN2XYtvCrlvObZneP
1XevafZtIIE4J4Pvi5neyb1sA3B/AJIIt19w/+rkjx+kYMvluGYtBLlQ/P0v/TBp
4OeneFCwcqa/rzHEzbzF56P9OZYT4r3RYOM7BqLhUOwyhtOoWTk=
=l4X3
-----END PGP SIGNATURE-----

--6qU/fYYiy7OeI3nj--