On Wed, Oct 20, 2021 at 9:27 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Wed, Oct 20, 2021 at 3:07 AM Alvaro Herrera <alvhe...@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/
copy_status_flags_v3.patch
Description: Binary data