Skip site navigation (1) Skip section navigation (2)

Re: sinval synchronization considered harmful

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Noah Misch <noah(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: sinval synchronization considered harmful
Date: 2011-07-27 17:30:47
Message-ID: CA+TgmoYMu3zgH7TqFSPVBW9LOWQPiBSMkG8ii26mtqfSHZHhUA@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Wed, Jul 27, 2011 at 12:55 PM, Noah Misch <noah(at)2ndquadrant(dot)com> wrote:
> This approach would work if a spinlock release constrained the global stores
> timeline.  It makes a weaker guarantee: all stores preceding the lock release
> in program order will precede it globally.  Consequently, no processor will
> observe the spinlock to be available without also observing all stores made by
> the last holding processor before that processor released it.  That guarantee
> is not enough for this sequence of events:
>
> Backend 0:
>        add a message for table "a"
>        store 5 => maxMsgNum
>        store true => timeToPayAttention
>        LWLockRelease(SInvalWriteLock)
> <plenty of time passes>
> Backend 2:
>        LOCK TABLE a;
>        load timeToPayAttention => true
> Backend 1:
>        add a message for table "b"
>        store 6 => maxMsgNum
>        SpinLockRelease(&vsegP->msgnumLock);
>        store true => timeToPayAttention
> Backend 2:
>        store false => timeToPayAttention
>        LWLockAcquire(SInvalReadLock, LW_SHARED)
>        load maxMsgNum => 5 [!]

Eh, how can this possibly happen?  You have to hold msgNumLock to to
set maxMsgNum and msgNumLock to read maxMsgNum.  If that's not enough
to guarantee that we never read a stale value, what is?

> I think a benchmark is in order, something like 900 idle connections and 80
> connections running small transactions that create a few temporary tables.  If
> that shows no statistically significant regression, then we're probably fine
> here.  I'm not sure what result to expect, honestly.

That's setting the bar pretty high.  I don't mind doing the
experiment, but I'm not sure that's the case we should be optimizing
for.

> What did you think of making the message number a uint64, removing wraparound
> handling, and retaining SISeg.msgnumLock for 32-bit only?  You could isolate the
> variant logic in READ_MSGNUM and WRITE_MSGNUM macros.

Well, what you really need to know is whether the platform has 8-byte
atomic stores, which doesn't seem trivial to figure out, plus you need
a memory fence.  If that's the only method of fixing this problem we
can agree on, I'm willing to work on it, but an
architecture-independent fix would be nicer.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

pgsql-hackers by date

Next:From: Florian PflugDate: 2011-07-27 17:37:31
Subject: Re: XMLATTRIBUTES vs. values of type XML
Previous:From: David FetterDate: 2011-07-27 17:17:44
Subject: Re: [COMMITTERS] pgsql: Add missing newlines at end of error messages

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group