On Thu, Nov 11, 2021 at 1:37 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Nov 11, 2021 at 3:07 PM Amit Kapila <amit.kapila16@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.
