Re: max_wal_senders must die

On Fri, Nov 12, 2010 at 11:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Bruce Momjian <bruce@momjian.us> writes:
>> Right.  I propose that we set max_wal_senders to unlimited when
>> wal_level = hot_standby.
>
> It's a memory allocation parameter ... you can't just set it to
> "unlimited", at least not without a nontrivial amount of work.

Yes.  This thread would benefit from less uneducated speculation and
more examination of what the parameter actually does.  I've looked at
how it's set up in the hopes of finding an easy fix and haven't come
up with anything all that wonderful yet.  In particular, there is this
code, that gets run on every commit (unless max_wal_senders is zero):

/* Wake up all walsenders */
void
WalSndWakeup(void)
{       int             i;
       for (i = 0; i < max_wal_senders; i++)               SetLatch(&WalSndCtl->walsnds[i].latch);
}

Notice that this code is able to iterate over all of the WAL senders
that might exist without taking any sort of lock.  You do NOT want to
unnecessarily iterate over the entire procarray here.  The obvious fix
is to keep an array that contains only the latches for the WAL senders
that actually exist at the moment, but then you have to insert a lock
acquisition and release in here, or risk backends failing to see an
update to the shared variable that identifies the end of the list,
which seems like a pretty bad idea too.

One idea I've had is that we might want to think about defining an
operation that is effectively "store, with a memory barrier".  For
example, on x86, this could be implemented using xchg.  I think if you
have a single-word variable in shared memory that is always updated
using a locked store, then individual backends should be able to read
it unlocked without risk of seeing a stale value.  So in the above
example you could have an array in shared memory and a variable
updated in the manner just described indicating how many slots of the
array are in use, and backends could iterate up to the end of the
array without needing a lock on the number of slots.  Of course, you
still have to figure out how to compact the array when a WAL sender
exits, but maybe that could be done with an extra layer of
indirection.

Come to think of it, I'm not really sure I understand what protects
SetLatch() against memory ordering hazards.  Is that actually safe?

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