On Thu, Jun 4, 2020 at 8:45 AM Andres Freund <andres@anarazel.de> wrote:
> On 2020-06-03 14:19:45 -0400, Tom Lane wrote:
> > In the second place, this coding seems to me to indicate serious
> > confusion about which lock is protecting what. In the above example,
> > if writtenUpto is only accessed through atomic operations then it
> > seems like we could just move the pg_atomic_read_u64 out of the
> > spinlock section; or if the spinlock is adequate protection then we
> > could just do a normal fetch. If we actually need both locks then
> > this needs significant re-thinking, IMO.
>
> Yea, it doesn't seem necessary at all that writtenUpto is read with the
> spinlock held. That's very recent:
>
> commit 2c8dd05d6cbc86b7ad21cfd7010e041bb4c3950b
> Author: Michael Paquier <michael@paquier.xyz>
> Date: 2020-05-17 09:22:07 +0900
>
> Make pg_stat_wal_receiver consistent with the WAL receiver's shmem info
>
> and I assume just was caused by mechanical replacement, rather than
> intentionally doing so while holding the spinlock. As far as I can tell
> none of the other writtenUpto accesses are under the spinlock.
Yeah. It'd be fine to move that after the spinlock release. Although
it's really just for informational purposes only, not for any data
integrity purpose, reading it before the spinlock acquisition would
theoretically allow it to appear to be (reportedly) behind
flushedUpto, which would be silly.