On Sun, 28 Aug 2022 at 20:38, Peter Geoghegan <p...@bowt.ie> wrote: > > On Tue, Aug 2, 2022 at 4:12 PM Peter Geoghegan <p...@bowt.ie> wrote: > > That is, we only need to make sure that the "multiXactCutoff must > > always be <= oldestMxact" invariant holds once, by checking for it > > directly. The same thing happens with OldestXmin/FreezeLimit. That > > seems like a simpler foundation. It's also a lot more logical. Why > > should the cutoff for freezing be held back by a long running > > transaction, except to the extent that it is strictly necessary to do > > so to avoid wrong answers (wrong answers seen by the long running > > transaction)? > > Anybody have any input on this? I'm hoping that this can be committed soon.
Apart from the message that this behaviour is changing, I'd prefer some more description in the commit message as to why this needs changing. Then, on to the patch itself: > + * XXX We don't do push back oldestMxact here, which is not ideal Do you intend to commit this marker, or is this leftover from the development process? > + if (*multiXactCutoff < FirstMultiXactId) [...] > + if (safeOldestMxact < FirstMultiXactId) [...] > + if (aggressiveMXIDCutoff < FirstMultiXactId) I prefer !TransactionId/MultiXactIdIsValid() over '< First [MultiXact/Transaction]Id', even though it is the same in functionality, because it clarifies the problem we're trying to solve. I understand that the use of < is pre-existing, but since we're touching this code shouldn't we try to get this new code up to current standards? Kind regards, Matthias van de Meent
