Date: Thu, 17 Feb 2022 10:23:37 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: "Bossart, Nathan" <bossartn@amazon.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	Maxim Orlov <orlovmg@gmail.com>, Amul Sul <sulamul@gmail.com>,
	Bruce Momjian <bruce@momjian.us>,
	Robert Haas <robertmhaas@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: O(n) tasks cause lengthy startups and checkpoints
Message-ID: <20220217182337.GA3247866@nathanxps13>
References: <83BE6E07-CEF5-445B-A8F9-2A2DD56F08CA@amazon.com>
 <CACG=ezagGrdsdKpqjUpF34f6PkEHUjqVeUmffQsFsye7h=QeEg@mail.gmail.com>
 <D71FD587-F027-442B-929D-7059184CA833@amazon.com>
 <CALj2ACVsYPxnQhp0=08aSzjdXPPbM5VXB2QDL8p-D_US1UNEqw@mail.gmail.com>
 <83A7426F-A7B6-4A10-A8F0-179AE30B871D@amazon.com>
 <20220211180249.GA1886051@nathanxps13>
 <20220217005057.GA3201038@nathanxps13>
 <20220217015052.2y3wxibeommk37ey@alap3.anarazel.de>
 <20220217041404.GA3243546@nathanxps13>
 <20220217065938.x2esfdppzypegn5j@alap3.anarazel.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220217065938.x2esfdppzypegn5j@alap3.anarazel.de>
Archived-At: <https://www.postgresql.org/message-id/20220217182337.GA3247866%40nathanxps13>
Precedence: bulk

On Wed, Feb 16, 2022 at 10:59:38PM -0800, Andres Freund wrote:
> On 2022-02-16 20:14:04 -0800, Nathan Bossart wrote:
>> >> -	while ((spc_de = ReadDirExtended(spc_dir, "pg_tblspc", LOG)) != NULL)
>> >> +	while (!ShutdownRequestPending &&
>> >> +		   (spc_de = ReadDirExtended(spc_dir, "pg_tblspc", LOG)) != NULL)
>> >
>> > Uh, huh? It strikes me as a supremely bad idea to have functions *silently*
>> > not do their jobs when ShutdownRequestPending is set, particularly without a
>> > huge fat comment.
>>
>> The idea was to avoid delaying shutdown because we're waiting for the
>> custodian to finish relatively nonessential tasks.  Another option might be
>> to just exit immediately when the custodian receives a shutdown request.
> 
> I think we should just not do either of these and let the functions
> finish. For the cases where shutdown really needs to be immediate
> there's, uhm, immediate mode shutdowns.

Alright.

>> > Why does this not open us up to new xid wraparound issues? Before there was a
>> > hard bound on how long these files could linger around. Now there's not
>> > anymore.
>>
>> Sorry, I'm probably missing something obvious, but I'm not sure how this
>> adds transaction ID wraparound risk.  These files are tied to LSNs, and
>> AFAIK they won't impact slots' xmins.
> 
> They're accessed by xid. The LSN is just for cleanup. Accessing files
> left over from a previous transaction with the same xid wouldn't be
> good - we'd read wrong catalog state for decoding...

Okay, that part makes sense to me.  However, I'm still confused about how
this is handled today and why moving cleanup to a separate auxiliary
process makes matters worse.  I've done quite a bit of reading, and I
haven't found anything that seems intended to prevent this problem.  Do you
have any pointers?

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com