Re: FlexLocks

On Wed, Nov 16, 2011 at 10:26 AM, Kevin Grittner
<Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
> For example, if these two macros were defined, predicate.c wouldn't
> have needed any modifications, and I suspect that is true of many
> other files (although possibly needing a few other macros):
>
> #define LWLockId FlexLockId
> #define LWLockHeldByMe(lock) FlexLockHeldByMe(lock)
>
> Particularly with the function call it seems like it's a mistake to
> assume that test will always be the same between LW locks and flex
> locks.  There may be a better way to do it than the above, but I
> think a worthy goal would be to impose zero source code changes on
> code which continues to use "traditional" lightweight locks.

Well, it would certainly be easy enough to add those macros, and I'm
not necessarily opposed to it, but I fear it could end up being a bit
confusing in the long run. If we adopt this infrastructure, then I
expect knowledge of different types of FlexLocks to gradually
propagate through the system. Now, you're always going to use
LWLockAcquire() and LWLockRelease() to acquire and release an LWLock,
but a FlexLockId isn't guaranteed to be an LWLockId - any given value
might also refer to a FlexLock of some other type. If we let everyone
continue to refer to those things as LWLockIds, then it seems like
only a matter of time before someone has a variable that's declared as
LWLockId but actually doesn't refer to an LWLock at all. I think it's
better to bite the bullet and do the renaming up front, rather than
having to think about it every time you modify some code that uses
LWLockId or LWLockHeldByMe and say to yourself, "oh, wait a minute, is
this really guaranteed to be an LWLock?"

For LWLockHeldByMe, a sensible compromise might be to add a function
that asserts that the FlexLockId passed as an argument is in fact
pointing to an LWLock, and then calls FlexLockHeldByMe() and returns
the result. That way you'd presumably noticed if you used the more
specific function when you needed the more general one (because,
hopefully, the regression tests would fail). But I'm not seeing any
obvious way of providing a similar degree of insulation against
abusing LWLockId.

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