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