Hi, On 2023-01-11 14:29:25 -0800, Peter Geoghegan wrote: > On Sat, Jan 7, 2023 at 7:25 PM Andres Freund <and...@anarazel.de> wrote: > > Probably a good idea, although it doesn't neatly fit right now. > > I'll leave it for now. > > Attached is v2, which changes things based on your feedback. Would > like to get this out of the way soon.
Makes sense. It's clearly an improvement. > + * We can't use TransactionIdDidAbort here because it won't treat > transactions > + * that were in progress during a crash as aborted by now. We determine that > + * transactions aborted/crashed through process of elimination instead. s/by now//? > * When using an MVCC snapshot, we rely on XidInMVCCSnapshot rather than > * TransactionIdIsInProgress, but the logic is otherwise the same: do not > diff --git a/src/backend/access/transam/transam.c > b/src/backend/access/transam/transam.c > index 3a28dcc43..7629904bb 100644 > --- a/src/backend/access/transam/transam.c > +++ b/src/backend/access/transam/transam.c > @@ -110,7 +110,8 @@ TransactionLogFetch(TransactionId transactionId) > * transaction tree. > * > * See also TransactionIdIsInProgress, which once was in this module > - * but now lives in procarray.c. > + * but now lives in procarray.c, as well as comments at the top of > + * heapam_visibility.c that explain how everything fits together. > * ---------------------------------------------------------------- > */ +1 Greetings, Andres Freund