On Tue, Jan 18, 2022 at 9:19 PM Andres Freund <andres@anarazel.de> wrote:
>
> > + LWLockAcquire(ProcArrayLock, LW_SHARED);
> > + lsn = ShmemVariableCache->finishedProcsLastCommitLSN;
> > + for (index = 0; index < ProcGlobal->allProcCount; index++)
> > + {
> > + XLogRecPtr procLSN = ProcGlobal->allProcs[index].lastCommitLSN;
> > + if (procLSN > lsn)
> > + lsn = procLSN;
> > + }
> > + LWLockRelease(ProcArrayLock);
>
> I think it'd be better to go through the pgprocnos infrastructure, so that
> only connected procs need to be checked.
>
> LWLockAcquire(ProcArrayLock, LW_SHARED);
> for (i = 0; i < arrayP->numProcs; i++)
> {
> int pgprocno = arrayP->pgprocnos[i];
> PGPROC *proc = &allProcs[pgprocno];
>
> if (proc->lastCommitLSN > lsn)
> lsn =proc->lastCommitLSN;
> }
>
>
> > diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
> > index a58888f9e9..2a026b0844 100644
> > --- a/src/include/storage/proc.h
> > +++ b/src/include/storage/proc.h
> > @@ -258,6 +258,11 @@ struct PGPROC
> > PGPROC *lockGroupLeader; /* lock group leader, if I'm a member */
> > dlist_head lockGroupMembers; /* list of members, if I'm a leader */
> > dlist_node lockGroupLink; /* my member link, if I'm a member */
> > +
> > + /*
> > + * Last transaction metadata.
> > + */
> > + XLogRecPtr lastCommitLSN; /* cache of last committed LSN */
> > };
>
> We do not rely on 64bit integers to be read/written atomically, just 32bit
> ones. To make this work for older platforms you'd have to use a
> pg_atomic_uint64. On new-ish platforms pg_atomic_read_u64/pg_atomic_write_u64
> end up as plain read/writes, but on older ones they'd do the necessarily
> locking to make that safe...
All right, here's an updated patch.
The final interface (new function or refactor the existing not to rely
on commit_ts) is still TBD (and I'd appreciate input on that from
Alvaro and others).
Thanks,
James Coleman