Simon Riggs <simon@2ndquadrant.com> writes:
> I've completed a review of all of the LWlocking in the backends. This is
> documented in the enclosed file. I would propose that we use this as
> comments in lwlock.h or in the README, if people agree.
I don't think that putting this list in as documentation is a smart
idea --- it would inevitably become out-of-date, and misleading
documentation is worse than none. Parts of it are out of date already
(which kinda proves my point considering that very little new development
has gone into the tree since September). Since anyone who's concerned
about a particular lock can grep for uses of it pretty easily, I think
we should just figure on them doing that.
> 2. CountActiveBackends() searches the whole of the proc array, even
> though it could stop when it gets to commit_siblings. Stopping once the
> heuristic has been determined seems like the best thing to do. A small
> patch to implement this is attached.
At the moment CountActiveBackends doesn't take the lock anymore, so
I'm thinking that changing this is not worthwhile.
> [ sinval lock management needs redesign ]
Yup it does.
> 4. WALWriteLock is acquired in Shared mode by bgwriter when it runs
> GetLastSegSwitchTime(). All other callers are Exclusive lockers, so the
> Shared request will queue like everybody else. WALWriteLock queue length
> can be long, so the bgwriter can get stuck for much longer than
> bgwriter_delay when it makes this call; this happens only when
> archive_timeout > 0 so probably has never shown up in any performance
> testing. XLogWrite takes info_lck also, so we can move the
> lastSegSwitchTime behind that lock instead. That way bgwriter need never
> wait on I/O, just spin for access to info_lck. Minor change.
This seems like a possibly reasonable thing to do; did you ever write
a patch for it?
> 5. ReadNewTransactionId() is only called now by GetNextXidAndEpoch(),
> but I can't find a caller of that anywhere in core or contrib. Can those
> now be removed?
No. It's needed by Slony.
> 6. David Strong talked about doing some testing to see if
> NUM_BUFFER_PARTITIONS should be increased above 16. We don't have any
> further information on that. Should we increase the value to 32 or 64?
Not without some testing.
regards, tom lane
