Re: pgsql: Delay commit status checks until freezing executes.

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Peter Geoghegan <pg@bowt.ie>
Cc: pgsql-hackers@postgresql.org
Date: 2023-01-04T06:33:25Z
Lists: pgsql-hackers
Hi,

On 2023-01-03 20:29:53 -0800, Peter Geoghegan wrote:
> On Tue, Jan 3, 2023 at 7:56 PM Andres Freund <andres@anarazel.de> wrote:
> > I don't know - I think there's a explicit comment somewhere, but I couldn't
> > find it immediately. There's a bunch of indirect references to in in
> > heapam_visibility.c, with comments like "it must have aborted or
> > crashed".
> 
> I think that that's a far cry from any kind of documentation...

Agreed - not sure if there never were docs, or whether they were accidentally
removed. This stuff has been that way for a long time.

I'd say a comment above TransactionIdDidAbort() referencing an overview
comment at the top of the file? I think it might be worth moving the comment
from heapam_visibility.c to transam.c?


> > The reason for the behaviour is that we do not have any mechanism for going
> > through the clog and aborting all in-progress-during-crash transactions. So
> > we'll end up with the clog for all in-progress-during-crash transaction being
> > zero / TRANSACTION_STATUS_IN_PROGRESS.
> 
> I find this astonishing. Why isn't there a prominent comment that
> advertises that TransactionIdDidAbort() just doesn't work reliably?

Arguably it works reliably, just more narrowly than one might think. Treating
"crashed transactions" as a distinct state from explicit aborts.

Greetings,

Andres Freund



Commits

  1. Improve TransactionIdDidAbort() documentation.

  2. Check that xmax didn't commit in freeze check.

  3. Delay commit status checks until freezing executes.

  4. Add page-level freezing to VACUUM.

  5. Set relfrozenxid to oldest extant XID seen by VACUUM.