Hi, On 2023-01-03 20:29:53 -0800, Peter Geoghegan wrote: > On Tue, Jan 3, 2023 at 7:56 PM Andres Freund <and...@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
