Re: elog(DEBUG2 in SpinLocked section.

On Tue, Jun 9, 2020 at 1:59 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > Removing some of these spinlocks and replacing them with LWLocks might
> > also be worth considering.
>
> When I went through the existing spinlock stanzas, the only thing that
> really made me acutely uncomfortable was the chunk in pg_stat_statement's
> pgss_store(), lines 1386..1438 in HEAD. In the first place, that's
> pushing the notion of "short straight-line code" well beyond reasonable
> bounds. Other processes could waste a fair amount of time spinning while
> the lock holder does all this arithmetic; not to mention the risk of
> exhausting one's CPU time-slice partway through. In the second place,
> a chunk of code this large could well allow people to make modifications
> without noticing that they're inside a spinlock, allowing future coding
> violations to sneak in.
>
> Not sure what we want to do about it though. An LWLock per pgss entry
> probably isn't gonna do. Perhaps we could take a cue from your old
> hack with multiplexed spinlocks, and map the pgss entries onto some
> fixed-size pool of LWLocks, figuring that the odds of false conflicts
> are small as long as the pool is bigger than MaxBackends.

I mean, what would be wrong with having an LWLock per pgss entry? If
you're worried about efficiency, it's no longer the case that an
LWLock uses a spinlock internally, so there's not the old problem of
doubling (plus contention) the number of atomic operations by using an
LWLock. If you're worried about space, an LWLock is only 16 bytes, and
the slock_t that we'd be replacing is currently at the end of the
struct so presumably followed by some padding.

I suspect that these days many of the places we're using spinlocks are
buying little of any value on the efficiency side, but making any
high-contention scenarios way worse. Plus, unlike LWLocks, they're not
instrumented with wait events, so you can't even find out that you've
got contention there without breaking out 'perf', not exactly a great
thing to have to do in a production environments.

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