Re: Slow standby snapshot

Hi,

On 2022-11-15 23:14:42 +0000, Simon Riggs wrote:
> On Tue, 15 Nov 2022 at 23:06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > BTW, while nosing around this code I came across this statement
> > (procarray.c, about line 4550 in HEAD):
> >
> > * ... To add XIDs to the array, we just insert
> > * them into slots to the right of the head pointer and then advance the head
> > * pointer. This wouldn't require any lock at all, except that on machines
> > * with weak memory ordering we need to be careful that other processors
> > * see the array element changes before they see the head pointer change.
> > * We handle this by using a spinlock to protect reads and writes of the
> > * head/tail pointers. (We could dispense with the spinlock if we were to
> > * create suitable memory access barrier primitives and use those instead.)
> > * The spinlock must be taken to read or write the head/tail pointers unless
> > * the caller holds ProcArrayLock exclusively.
> >
> > Nowadays we've *got* those primitives. Can we get rid of
> > known_assigned_xids_lck, and if so would it make a meaningful
> > difference in this scenario?

Forgot to reply to this part:

I'm confused by the explanation of the semantics of the spinlock:

"The spinlock must be taken to read or write the head/tail pointers
unless the caller holds ProcArrayLock exclusively."

makes it sound like it'd be valid to modify the KnownAssignedXids without
holding ProcArrayLock exclusively. Doesn't that contradict only needing the
spinlock because of memory ordering?

And when would it be valid to do any modifications of KnownAssignedXids
without holding ProcArrayLock exclusively? Concurrent readers of
KnownAssignedXids will operate on a snapshot of head/tail (the spinlock is
only ever held to query them). Afaict any such modification would be racy,
because subsequent modifications of KnownAssignedXids could overwrite contents
of KnownAssignedXids that another backend is in the process of reading, based
on the stale snapshot of head/tail.

To me it sounds like known_assigned_xids_lck is pointless and the talk about
memory barriers a red herring, since all modifications have to happen with
ProcArrayLock held exlusively and all reads with ProcArrayLock held in share
mode. It can't be legal to modify head/tail or the contents of the array
outside of that. And lwlocks provide sufficient barrier semantics.

> I think you could do that *as well*, since it does act as an overhead
> but that is not related to the main issues

I think it might be a bigger effect than one might immediately think. Because
the spinlock will typically be on the same cacheline as head/tail, and because
every spinlock acquisition requires the cacheline to be modified (and thus
owned mexlusively) by the current core, uses of head/tail will very commonly
be cache misses even in workloads without a lot of KAX activity.

Greetings,

Andres Freund