From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Kevin Grittner <kgrittn(at)gmail(dot)com> |
Cc: | Kevin Grittner <kgrittn(at)postgresql(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "pgsql-committers(at)postgresql(dot)org" <pgsql-committers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Re: pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold < |
Date: | 2016-04-12 18:56:25 |
Message-ID: | 20160412185625.g7ixnlpwaqvsqt7v@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers 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(at)anarazel(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Kevin Grittner | 2016-04-12 18:57:26 | Re: pgsql: Add the "snapshot too old" feature |
Previous Message | Kevin Grittner | 2016-04-12 18:44:00 | Re: pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold < |
From | Date | Subject | |
---|---|---|---|
Next Message | Kevin Grittner | 2016-04-12 18:57:26 | Re: pgsql: Add the "snapshot too old" feature |
Previous Message | Robert Haas | 2016-04-12 18:44:34 | Re: Lets (not) break all the things. Was: [pgsql-advocacy] 9.6 -> 10.0 |