Re: dynamic shared memory and locks - Mailing list pgsql-hackers

On Tue, Jan 21, 2014 at 11:37 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> One idea I just had is to improve the dsm_toc module so that it can
> optionally set up a tranche of lwlocks for you, and provide some
> analogues of RequestAddinLWLocks and LWLockAssign for that case.  That
> would probably make this quite a bit simpler to use, at least for
> people using it with dynamic shared memory.  But I think that's a
> separate patch.

I played with this a bit today and it doesn't actually seem to
simplify things very much.  The backend that creates the DSM needs to
do this:
   lwlocks = shm_toc_allocate(toc, sizeof(LWLockPadded) * nlwlocks);   for (i = 0; i < nlwlocks; ++i)
LWLockInitialize(&lwlocks[i].lock,tranche_id);
 

Since that's all of three lines, encapsulating it doesn't look all
that helpful.  Then each backend needs to do something like this:
   static LWLockTranche mytranche;   mytranche.name = "some descriptive module name";   mytranche.array_base = lwlocks;
 mytranche.array_stride = sizeof(LWLockPadded);   LWLockRegisterTranche(tranche_id, &mytranche);
 

That's not a lot of code either, and there's no obvious way to reduce
it much by hooking into shm_toc.

I thought maybe we needed some cleanup when the dynamic shared memory
segment is unmapped, but looks like we really don't.  One can do
something like LWLockRegisterTranche(tranche_id, NULL) for debugging
clarity, but it isn't strictly needed; one can release all lwlocks
from the tranche or assert that none are held, but that should really
only be a problem if the user does something like
LWLockAcquire(lock_in_the_segment); dsm_detach(seg), because error
recovery starts by releasing *all* lwlocks.  And if the user does it
explicitly, I'm kinda OK with that just seg faulting.  After all, the
user could equally well have done dsm_detach(seg);
LWLockAcquire(lock_in_the_segment) and there's no way at all to
cross-check for that sort of mistake.

I do see one thing about the status quo that does look reasonably
annoying: the code expects that tranche IDs are going to stay
relatively small.  For example, consider a module foobar that uses DSM
+ LWLocks.  It won't do at all for each backend, on first use of the
foobar module, to do LWLockNewTrancheId() and then reuse that
tranche_id repeatedly for each new DSM - because over a system
lifetime of months, tranche IDs could grow into the millions, causing
LWLockTrancheArray to get really big (and eventually repalloc will
fail).  Rather, the programmer needs to ensure that
LWLockNewTrancheId() gets called *once per postmaster lifetime*,
perhaps by allocating a chunk of permanent shared memory and using
that to store the tranche_id that should be used each time an
individual backend fires up a DSM.  Considering that one of the goals
of dynamic shared memory is to allow modules to be loaded after
postmaster startup and still be able to do useful stuff, that's kind
of obnoxious.  I don't have a good idea what to do about it, though;
making LWLockTrancheArray anything other than a flat array looks
likely to slow down --enable-dtrace builds unacceptably.

Bottom line, I guess, is that I don't currently have any real idea how
to make this any better than it already is.  Maybe that will become
more clear as this facility (hopefully) acquires some users.

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