On Sun, May 15, 2022 at 12:29 AM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > > On 2022-Apr-20, Masahiko Sawada wrote: > > > > MyProc->statusFlags = (MyProc->statusFlags & ~PROC_XMIN_FLAGS) | > > > (proc->statusFlags & PROC_XMIN_FLAGS); > > > > > > Perhaps the latter is more future-proof. > > > Copying only xmin-related flags in this way also makes sense to me and > > there is no problem at least for now. A note would be that when we > > introduce a new flag that needs to be copied in the future, we need to > > make sure to add it to PROC_XMIN_FLAGS so it is copied. Otherwise a > > similar issue we fixed by 0f0cfb494004befb0f6e could happen again. > > OK, done this way -- patch attached.
Thank you for updating the patch. > > Reading the comment I wrote about it, I wonder if flags > PROC_AFFECTS_ALL_HORIZONS and PROC_IN_LOGICAL_DECODING should also be > included. I think the only reason we don't care at this point is that > walsenders (logical or otherwise) do not engage in snapshot copying. > But if we were to implement usage of parallel workers sharing a common > snapshot to do table sync in parallel, then it ISTM it would be > important to copy at least the latter. Not sure there are any cases > were we might care about the former. Yeah, it seems to be inconsistent between the comment (and the new name) and the flags actually included. I think we can include all xmin-related flags to PROC_XMIN_FLAGS as the comment says. That way, it would be useful also for other use cases, and I don't see any downside for now. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/