On 10/9/14, 4:57 PM, Andres Freund wrote:
If you modify either, you better grep for them... I don't think that's
going to happen anyway. Requiring it during startup would mean exposing
SHARED_LOCK_MASK outside of lwlock.c which'd be ugly. We could possibly
stick a StaticAssert() someplace in lwlock.c.
Ahh, yeah, exposing it would be ugly.
I just get the heeby-jeebies when I see assumptions like this though. I fear
there's a bunch of cases where changing something will break a completely
unrelated part of the system with no warning.
Maybe add an assert() to check it?
And no, it's not comparable at all to MyProc != NULL - the lwlock code
initially*does* run when MyProc isn't setup. We just better not
conflict against any other lockers at that stage.
Ahh, can you maybe add that detail to the comment? That wasn't clear to me.
> >+/*
> >+ * Internal function that tries to atomically acquire the lwlock in the
passed
> >+ * in mode.
> >+ *
> >+ * This function will not block waiting for a lock to become free - that's
the
> >+ * callers job.
> >+ *
> >+ * Returns true if the lock isn't free and we need to wait.
> >+ *
> >+ * When acquiring shared locks it's possible that we disturb an exclusive
> >+ * waiter. If that's a problem for the specific user, pass in a valid
pointer
> >+ * for 'potentially_spurious'. Its value will be set to true if we possibly
> >+ * did so. The caller then has to handle that scenario.
> >+ */
> >+static bool
> >+LWLockAttemptLock(LWLock* lock, LWLockMode mode, bool *potentially_spurious)
>
>We should invert the return of this function. Current code returns
>true if the lock is actually acquired (see below), and I think that's
>true of other locking code as well. IMHO it makes more sense that way,
>plus consistency is good.
I don't think so. I've wondered about it as well, but the way the
function is used its more consistent imo if it returns whether we must
wait. Note that it's not an exported function.
ISTM that a function attempting a lock would return success, not failure. Even
though it's internal now it could certainly be made external at some point in
the future. But I suppose it's ultimately a matter of preference...
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
