On Wed, Oct 20, 2021 at 9:27 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Oct 20, 2021 at 3:07 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >
> > On 2021-Oct-19, Alvaro Herrera wrote:
> >
>
> Thank you for the comment.
>
> > > Hmm, I think this should happen before the transaction snapshot is
> > > established in the worker; perhaps immediately after calling
> > > StartParallelWorkerTransaction(), or anyway not after
> > > SetTransactionSnapshot. In fact, since SetTransactionSnapshot receives
> > > a 'sourceproc' argument, why not do it exactly there? ISTM that
> > > ProcArrayInstallRestoredXmin() is where this should happen.
> >
> > ... and there is a question about the lock strength used for
> > ProcArrayLock. The current routine uses LW_SHARED, but there's no
> > clarity that we can modify proc->statusFlags and ProcGlobal->statusFlags
> > without LW_EXCLUSIVE.
> >
> > Maybe we can change ProcArrayInstallRestoredXmin so that if it sees that
> > proc->statusFlags is not zero, then it grabs LW_EXCLUSIVE (and copies),
> > otherwise it keeps using LW_SHARED as it does now (and does not copy.)
>
> Initially, I've considered copying statusFlags in
> ProcArrayInstallRestoredXmin() but I hesitated to do that because
> statusFlags is not relevant with xmin and snapshot stuff. But I agree
> that copying statusFlags should happen before restoring the snapshot.
>
> If we copy statusFlags in ProcArrayInstallRestoredXmin() there is
> still little window that the restored snapshot holds back the oldest
> xmin?
That's wrong, I'd misunderstood.
I agree to copy statusFlags in ProcArrayInstallRestoredXmin(). I've
updated the patch accordingly.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
