Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold < - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <
Date
Msg-id 20160412185625.g7ixnlpwaqvsqt7v@alap3.anarazel.de
Whole thread Raw
In response to Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <  (Kevin Grittner <kgrittn@gmail.com>)
Responses Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <
Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <
List pgsql-hackers
On 2016-04-12 13:44:00 -0500, Kevin Grittner wrote:
> On Tue, Apr 12, 2016 at 12:38 PM, Andres Freund <andres@anarazel.de> wrote:
> > On 2016-04-12 16:49:25 +0000, Kevin Grittner wrote:
> >> On a big NUMA machine with 1000 connections in saturation load
> >> there was a performance regression due to spinlock contention, for
> >> acquiring values which were never used.  Just fill with dummy
> >> values if we're not going to use them.
> >
> > FWIW, I could see massive regressions with just 64 connections.
>
> With what settings?

You mean pgbench or postgres? The former -M prepared -c 64 -j 64 -S. The
latter just a large enough shared buffers to contains the scale 300
database, and adapted maintenance_work_mem. Nothing special.


>  With or without the patch to avoid the locks when off?

Without. Your commit message made it sound like you need unrealistic or
at least unusual numbers of connections, and that's afaics not the case.


> > I'm a bit scared of having an innoccuous sounding option regress things
> > by a factor of 10. I think, in addition to this fix, we need to actually
> > solve the scalability issue here to a good degree.  One way to do so is
> > to apply the parts of 0001 in
> > http://archives.postgresql.org/message-id/20160330230914.GH13305%40awork2.anarazel.de
> > defining PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY and rely on that. Another
> > to apply the whole patch and simply put the lsn in an 8 byte atomic.
>
> I think that we are well due for atomic access to aligned 8-byte
> values.  That would eliminate one potential hot spot in the
> "snapshot too old" code, for sure.

I'm kinda inclined to apply that portion (or just the whole patch with
the spurious #ifdef 0 et al fixed) into 9.6; and add the necessary
checks in a few places. Because I really think this is likely to hit
unsuspecting users.

FWIW, accessing a frequently changing value from a significant number of
connections, at a high frequency, isn't exactly free without a spinlock
either. But it should be much less bad.

Greetings,

Andres Freund


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Lets (not) break all the things. Was: [pgsql-advocacy] 9.6 -> 10.0
Next
From: Kevin Grittner
Date:
Subject: Re: pgsql: Add the "snapshot too old" feature