Re: Make relation_openrv atomic wrt DDL

On Mon, Jun 13, 2011 at 1:12 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> That might be a start, but it's not a complete replacement for the global
> counter.  AcceptInvalidationMessages() is actually called in LockRelationOid(),
> but the comparison needs to happen a level up in RangeVarLockRelid().  So, we
> would be adding encapsulation in one place to lose it in another.  Also, in the
> uncontended case, the patch only calls AcceptInvalidationMessages() once per
> relation_openrv.  It compares the counter after that call with a counter as the
> last caller left it -- RangeVarLockRelid() doesn't care who that caller was.

Hmm, OK.

>> Taking that a bit further, what if we put that counter in
>> shared-memory?  After writing new messages into the queue, a writer
>> would bump this count (only one process can be doing this at a time
>> because SInvalWriteLock is held) and memory-fence.  Readers would
>> memory-fence and then read the count before acquiring the lock.  If it
>> hasn't changed since we last read it, then don't bother acquiring
>> SInvalReadLock, because no new messages have arrived.  Or maybe an
>> exact multiple of 2^32 messages have arrived, but there's probably
>> someway to finesse around that issue, like maybe also using some kind
>> of memory barrier to allow resetState to be checked without the lock.
>
> This probably would not replace a backend-local counter of processed messages
> for RangeVarLockRelid()'s purposes.  It's quite possibly a good way to reduce
> SInvalReadLock traffic, though.
>
> Exact multiples of 2^32 messages need not be a problem, because the queue is
> limited to MAXNUMMESSAGES (4096, currently).  I think you will need to pack into
> one 32-bit value all data each backend needs to decide whether to proceed with
> the full process.  Given that queue offsets fit into 13 bits (easily reduced to
> 12) and resetState is a bit, that seems practical enough at first glance.

I was imagining one shared global counter, not one per backend, and
thinking that each backend could do something like:

volatile uint32 *the_global_counter = &global_counter;
uint32 latest_counter;
mfence();
latest_counter = *the_global_counter;
if (latest_counter != previous_value_of_global_counter || myprocstate->isReset)
really_do_it();
previous_value_of_global_counter = latest_counter;

I'm not immediately seeing why that wouldn't work for your purposes as well.

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