Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <

On 2016-04-13 11:08:21 -0300, Alvaro Herrera wrote:
> The patch being proposed for commit is fiddly architecture-specific
> stuff which is likely to destabilize the tree for quite some time, and
> cause lots of additional work to Andres and anyone else likely to work
> on such low-level details, such as Robert, both of which already have
> plenty to do.

Personally I think this is an 9.6 open-item, and primarily Kevin has to
work on it.

Note that there really shouldn't be too much fiddly bits, we've already
had 64bit atomics, just no fallback. This is just copying the fallback
code from 32bit atomics to 64bit atomics.

But what I'm actually proposing isn't even using the 64bit atomics from
that patch, just to add

--- a/src/include/port/atomics/arch-ppc.h
+++ b/src/include/port/atomics/arch-ppc.h
@@ -24,3 +24,6 @@#define pg_read_barrier_impl()        __asm__ __volatile__ ("lwsync" : : : "memory")#define
pg_write_barrier_impl()       __asm__ __volatile__ ("lwsync" : : : "memory")#endif
 
+
+/* per architecture manual doubleword accesses have single copy atomicity */
+#define PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY

to the appropriate files (ia64, ppc, x86) and then add an #ifndef
PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY to GetXLogInsertRecPtr's acquisition
of the spinlock. I.e.
#ifndef PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITYSpinLockAcquire(&Insert->insertpos_lck);
#endif       current_bytepos = Insert->CurrBytePos;
#ifndef PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITYSpinLockRelease(&Insert->insertpos_lck);
#endif

not because I think it's perfectly pretty that way, but because it's
very easy to demonstrate that there's no regressions for anybody.

> The regression only seems to show up if you turn it on and have a
> crazily high rate of read-only transactions.  I think this can wait
> for 9.7.

I don't think 120k read tps is all that high anymore these days. And you
can easily create scenarios that are *much* worse than pgbench. E.g. a
loop in a volatile plpgsql function will acquire 

Greetings,

Andres Freund