From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: dynamic shared memory and locks |
Date: | 2014-01-05 19:47:28 |
Message-ID: | 20140105194728.GC18220@alap2.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2014-01-05 14:06:52 -0500, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > For what it's worth, my vote is currently for #2. I can't think of
> > many interesting to do with dynamic shared memory without having at
> > least spinlocks, so I don't think we'd be losing much. #1 seems
> > needlessly unfriendly, #3 seems like a lot of work for not much, and
> > #4 seems excessive at least as a solution to this particular problem,
> > though there may be other arguments for it. What do others think?
>
> I agree with this position. There may be some good reason to drop
> --disable-spinlocks altogether in future, but DSM isn't a sufficient
> excuse.
Agreed that DSM isn't sufficient cause. The reasons for removing it for
future reasons I see are:
* It's not tested at all and it has been partially broken for
significants of time. Afair Heikki just noticed the breakage
accidentally when adding enough new spinlocks recently.
* It's showed up as problematic in a couple of patches while adding not
much value (at least dsm, atomic ops, afair some others)
* It's slow enough that it's not of a practical value.
* Implementing simple support for spinlocks on realistic platforms isn't
hard these days due to compiler intrinsics.
* The platforms that don't have a barrier implementation will rely on
spinlocks. And for correctness those spinlocks should employ
barriers. That might be more of an argument for getting rid of that
fallback tho.
> > I think we're also going to want to be able to create LWLocks in
> > dynamic shared memory: some critical sections won't be short enough or
> > safe enough to be protected by spinlocks. At some level this seems
> > easy: change LWLockAcquire and friends to accept an LWLock * rather
> > than an LWLockId, and similarly change held_lwlocks[] to hold LWLock
> > pointers rather than LWLockIds.
>
> I seem to recall that there was some good reason for keeping all the
> LWLocks in an array, back when the facility was first designed.
> I'm too lazy to research the point right now, but you might want to
> go back and look at the archives around when lwlock.c was written.
Your proposal is at
http://www.postgresql.org/message-id/1054.1001520600@sss.pgh.pa.us -
afaics there hasn't been much discussion about implementation details at
that level.
> In the end, though, that decision was taken before we were concerned
> about being able to add new LWLocks on the fly, which is what this is
> really about (whether they're stored in DSM or not is a secondary point).
> The pressure for that has gotten strong enough that it may be time to
> change the tradeoff.
I personally find the sharing of a cacheline between a buffer headers
spinlock and the lwlock much more interesting than DSM...
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Mark Dilger | 2014-01-05 19:51:16 | Re: In-core regression tests for replication, cascading, archiving, PITR, etc. Michael Paquier |
Previous Message | Fabrizio Mello | 2014-01-05 19:46:14 | Re: [PATCH] Support for pg_stat_archiver view |