Re: LWLockWaitUntilFree (was: Group commit, revised)

On Fri, Jan 27, 2012 at 8:35 AM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> I think you should break this off into a new function,
>> LWLockWaitUntilFree(), rather than treating it as a new LWLockMode.
>> Also, instead of adding lwWaitOnly, I would suggest that we generalize
>> lwWaiting and lwExclusive into a field lwWaitRequest, which can be set
>> to 1 for exclusive, 2 for shared, 3 for wait-for-free, etc.  That way
>> we don't have to add another boolean every time someone invents a new
>> type of wait - not that that should hopefully happen very often.  A
>> side benefit of this is that you can simplify the test in
>> LWLockRelease(): keep releasing waiters until you come to an exclusive
>> waiter, then stop.  This possibly saves one shared memory fetch and
>> subsequent test instruction, which might not be trivial - all of this
>> code is extremely hot.
>
> Makes sense. Attached is a new version, doing exactly that.

I think I recommended a bad name for this function. It's really
LWLockAcquireOrWaitUntilFree. Can we rename that?

We might also want to consider what all the behaviors are that we
might want here. It strikes me that there are two possible actions
when a lock is immediately available (acquire or noop) and three
possible actions when it isn't (wait-then-acquire, wait-then-noop, or
immediate-noop), for a total of six possible combinations:

acquire/wait-then-acquire = LWLockAcquire
acquire/wait-then-noop = what you just added
acquire/immediate-noop = LWLockConditionalAcquire
noop/wait-then-acquire
noop/wait-then-noop
noop/immediate-noop

The last one is clearly pointless, since you don't need a lock at all
to do nothing. But is there a use case for either of the other two?
I can't immediately think of any reason why we'd want
noop/wait-then-acquire but noop/wait-then-noop seems potentially
useful (that's what I originally thought LWLockWaitUntilFree was going
to do). For example, if for some reason you wanted all the WAL
flushes to happen in one process, you could have everyone else use
that primitive to sleep, and then recheck whether enough flushing had
happened (I don't actually think that's better; it's not very robust
against WAL writer going away). Or, for ProcArrayLock, you might (as
you proposed previously) we could mark our XID with a removal sequence
number in lieu of actually removing it, and then wait for
ProcArrayLock to be free before overwriting it with a new XID. Since
we're out of time for 9.2 I don't think we probably want to devote too
much time to experimenting with this right now, but it might be
interesting to play around with it next cycle.

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

On Fri, Feb 3, 2012 at 12:57 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> I think I recommended a bad name for this function.  It's really
> LWLockAcquireOrWaitUntilFree.  Can we rename that?
>
> We might also want to consider what all the behaviors are that we
> might want here.  It strikes me that there are two possible actions
> when a lock is immediately available (acquire or noop) and three
> possible actions when it isn't (wait-then-acquire, wait-then-noop, or
> immediate-noop), for a total of six possible combinations:
>
> acquire/wait-then-acquire = LWLockAcquire
> acquire/wait-then-noop = what you just added
> acquire/immediate-noop = LWLockConditionalAcquire
> noop/wait-then-acquire
> noop/wait-then-noop
> noop/immediate-noop
>
> The last one is clearly pointless, since you don't need a lock at all
> to do nothing.  But is there a use case for either of the other two?

You've only considered things with the same mode on each side of the
"/". One thing that I've wanted is:
acquire-exclusive-immediate-and-return-true/wait-then-acquire-shared-and-return-false.

The idea was that there is something that I need to verify has been
done, but it doesn't matter who did it. To do the work, I need
exclusive, but to verify that the work has already been done I only
need shared. So, if I don't get the exclusive lock, then by the time
I can get it the work has probably been done by someone else so I only
need a shared lock to verify that.

Come to think of it, I think what I wanted my behavior for was for
this very thing that inspired this WaitUntilFree, improving
group-commit. But I see in this case the verification is being done
under a spinlock rather than a shared LWLock. I don't know if there
is any case left over for my proposed behavior, or if spinlock
verification would always be easy to arrange so as to make it
pointless.

Cheers,

Jeff

On 03.02.2012 22:57, Robert Haas wrote:
> I think I recommended a bad name for this function. It's really
> LWLockAcquireOrWaitUntilFree. Can we rename that?

Agreed, that's better. Although quite long. "LWLockAcquireOrWait" perhaps?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, Feb 7, 2012 at 3:48 AM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> On 03.02.2012 22:57, Robert Haas wrote:
>>
>> I think I recommended a bad name for this function.  It's really
>> LWLockAcquireOrWaitUntilFree.  Can we rename that?
>
> Agreed, that's better. Although quite long. "LWLockAcquireOrWait" perhaps?

Sure.

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