On Wed, Apr 8, 2026 at 7:30 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Tue, Apr 7, 2026 at 6:55 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> >
> > On Tue, Apr 7, 2026 at 9:46 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > >
> > > On Tue, Apr 7, 2026, 16:18 Andres Freund <andres@anarazel.de> wrote:
> > >>
> > >> Hi,
> > >>
> > >> On 2026-04-07 21:05:40 +0800, Xuneng Zhou wrote:
> > >> > I’ve posted two patches. The first fixes the duplication issue
> > >> > reported by Andres and is fairly straightforward. The second turned
> > >> > out to be more complex than expected, and I’m still working through
> > >> > possible solutions. Feedback or alternative approaches would be very
> > >> > helpful.
> > >> > I also spent some time drafting a patch to address the memory ordering
> > >> > issue and will post it later.
> > >>
> > >> I propose quickly applying a minimal patch like the attached, to get the test
> > >> performance back to normal.
> > >>
> > >> Will do so unless somebody protests within in one CI cycle and one coffee.
> > >
> > >
> > > I would be able to review them only after several hours. But +1 for applying now to get rid of buildfarm
slowdown.
> >
> > Here is a patch addressing the memory order issue reported earlier.
>
> I agree to change in WaitLSNWakeup(), memory barrier looks necessary there.
> Regarding GetCurrentLSNForWaitType(), I don't think barrier is needed
> here, nor think it makes things clearer. I think it would be enough
> to comment that LWLock operations in addLSNWaiter()/deleteLSNWaiter()
> provide necessary barriers.
>
I’m fine with that if Andres has no objection. That said, callers of
WaitLSNWakeup except STANDBY_WRITE do acquire locks before the wakeup.
I’m not sure whether a memory barrier is required for all of them,
though placing the barrier inside WaitLSNWakeup would make the
handling less scattered.
--
Best,
Xuneng
