On Thu, Nov 11, 2021 at 1:37 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Thu, Nov 11, 2021 at 3:07 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > I think the disadvantage I see by not > > doing in ProcArrayInstallRestoedXmin is that we need to take procarray > > lock twice (once in exclusive mode and then in shared mode) so doing > > it in ProcArrayInstallRestoedXmin is beneficial from that angle. > > Right. I thought that this overhead is also negligible in this > context. If that’s right, it’d be better to do it in a separate > function from the clearness point of view. Also if we raise the lock > level in ProcArrayInstallRestoredXmin(), a caller of the function who > wants just to set xmin will end up acquiring an exclusive lock. Which > is unnecessary for the caller. >
As mentioned by Andres, ProcArrayInstallRestoredXmin() happens in an expensive context apart from this which is while creating logical replication, so the cost might not matter but I see your point about clarity. Basically, this function can get called from two different code paths i.e creation of logical replication slot and parallel worker startup but as of today we need it only in the latter case, so it is better to it in that code path (after calling StartParallelWorkerTransaction()). I think we can do that way unless Alvaro thinks otherwise as he had proposed to do it in ProcArrayInstallRestoredXmin(). Alvaro, others, do you favor any particular way to deal with this case? -- With Regards, Amit Kapila.
