Re: LogwrtResult contended spinlock

From: Andres Freund <andres(at)anarazel(dot)de>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>
Subject: Re: LogwrtResult contended spinlock
Date: 2021-01-30 02:02:11
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2021-01-29 12:40:18 -0300, Alvaro Herrera wrote:
> On 2020-Aug-31, Andres Freund wrote:
> > Wouldn't the better fix here be to allow reading of individual members without a lock? E.g. by wrapping each in a 64bit atomic.
> So I've been playing with this and I'm annoyed about having two
> datatypes to represent Write/Flush positions:
> typedef struct XLogwrtRqst
> {
> XLogRecPtr Write; /* last byte + 1 to write out */
> XLogRecPtr Flush; /* last byte + 1 to flush */
> } XLogwrtRqst;
> typedef struct XLogwrtResult
> {
> XLogRecPtr Write; /* last byte + 1 written out */
> XLogRecPtr Flush; /* last byte + 1 flushed */
> } XLogwrtResult;
> Don't they look, um, quite similar? I am strongly tempted to remove
> that distinction, since it seems quite pointless, and introduce a
> different one:

Their spelling drives me nuts. Like, one is *Rqst, the other *Result?

> Now, I do wonder if there's a point in keeping LogwrtResult as a local
> variable at all; maybe since the ones in shared memory are going to use
> unlocked access, we don't need it anymore? I'd prefer to defer that
> decision to after this patch is done, since ISTM that it'd merit more
> careful benchmarking.

I think doing that might be a bit harmful - we update LogwrtResult
fairly granularly in XLogWrite(). Doing that in those small steps in
shared memory will increase the likelihood of cache misses in other


Andres Freund

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2021-01-30 02:03:58 Re: Thoughts on "killed tuples" index hint bits support on standby
Previous Message Tom Lane 2021-01-30 01:53:49 Re: Should we make Bitmapsets a kind of Node?