From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Andres Freund <andres@anarazel.de>
Cc: Jacob Speidel <jacob@extrahop.com>
Cc: Will Mortensen <will@extrahop.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17973: Reinit of pgstats entry for dropped DB can break autovacuum daemon
Date: Fri, 1 Sep 2023 08:20:24 -0700
Message-ID: <20230901152024.GA3175143@nathanxps13> (raw)
In-Reply-To: <ZPFa/bED0YZc0oE3@paquier.xyz>
References: <20230614215744.ddeuii7jcojbp3yv@awork3.anarazel.de>
<20230614223012.cfyzxyfnt3ggqkt4@awork3.anarazel.de>
<CAMpnoC5=njD8tLG9q8XQZKi=1SfLehjVPmLJC+pktzGdfZEgoA@mail.gmail.com>
<CA+pD2MKw6FwU21efw4+kAziFXWB+cnfxiE1824COmp=0x+jwBQ@mail.gmail.com>
<ZNbRByjoQlUtTkTb@paquier.xyz>
<20230812004812.4yev47iili57u4hc@awork3.anarazel.de>
<ZNlxRmI15e1BtCR6@paquier.xyz>
<ZPE5qXLuMs+/vx9H@paquier.xyz>
<20230901025613.GB3097511@nathanxps13>
<ZPFa/bED0YZc0oE3@paquier.xyz>
On Fri, Sep 01, 2023 at 12:31:09PM +0900, Michael Paquier wrote:
> On Thu, Aug 31, 2023 at 07:56:13PM -0700, Nathan Bossart wrote:
>> On Fri, Sep 01, 2023 at 10:08:57AM +0900, Michael Paquier wrote:
>>> + /*
>>> + * Now that we rechecked, we are certain to be connected to a database and
>>> + * thus can set MyDatabaseId.
>>> + */
>>> + MyDatabaseId = dboid;
>>
>> Perhaps we should expand this comment to make it clear that some code
>> depends on this to avoid races with concurrent database drops/renames.
>
> Yeah, this could be a bit more detailed, mentioning the dependency
> with the pgstat shutdown callback. How about appending the following
> sentence in a second paragraph, say:
> "Setting MyDatabaseId after rechecking the existence of the database
> matters for instance with the shutdown callback of the shared
> statistics, as pgstat_report_stat() will update database statistics
> depending on MyDatabaseId."
Something along those lines seems fine to me. FWIW this is what I had in
my head:
It is important that MyDatabaseId only be set once we are sure that the
target database can no longer be concurrently dropped or renamed. Some
code depends on this. For example, without this guarantee,
pgstat_update_dbstats() could create entries for databases that were
just dropped, which will lead to problems down the road.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
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-bugs@postgresql.org
Cc: nathandbossart@gmail.com, michael@paquier.xyz, andres@anarazel.de, jacob@extrahop.com, will@extrahop.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17973: Reinit of pgstats entry for dropped DB can break autovacuum daemon
In-Reply-To: <20230901152024.GA3175143@nathanxps13>
* 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