Assert(LWLockHeldByMeInMode(lock, LW_EXCLUSIVE))

Hi hackers,

Several times now when reading, debugging and writing code I've wished
that LWLockHeldByMe assertions specified the expected mode, especially
where exclusive locking is required.

What do you think about something like the attached? See also an
example of use. I will add this to the next commitfest.

--
Thomas Munro
http://www.enterprisedb.com

On 18 June 2016 at 11:28, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
wrote:

> Hi hackers,
>
> Several times now when reading, debugging and writing code I've wished
> that LWLockHeldByMe assertions specified the expected mode, especially
> where exclusive locking is required.
>
> What do you think about something like the attached? See also an
> example of use. I will add this to the next commitfest.

I've wanted this before too, and was surprised it wasn't present. TBH I
assumed there was a technical reason it wasn't and didn't investigate
further because I just assumed it'd have been added with the original
LWLockHeldByMe if it were simple.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, Jun 20, 2016 at 3:43 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> On 18 June 2016 at 11:28, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
> wrote:
>> Several times now when reading, debugging and writing code I've wished
>> that LWLockHeldByMe assertions specified the expected mode, especially
>> where exclusive locking is required.
>>
>> What do you think about something like the attached? See also an
>> example of use. I will add this to the next commitfest.
>
> I've wanted this before too, and was surprised it wasn't present. TBH I
> assumed there was a technical reason it wasn't and didn't investigate
> further because I just assumed it'd have been added with the original
> LWLockHeldByMe if it were simple.

Before ab5194e6f (25 December 2014) held_lwlocks didn't record the mode.

--
Thomas Munro
http://www.enterprisedb.com

On 18 June 2016 at 04:28, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> Hi hackers,
>
> Several times now when reading, debugging and writing code I've wished
> that LWLockHeldByMe assertions specified the expected mode, especially
> where exclusive locking is required.
>
> What do you think about something like the attached? See also an
> example of use. I will add this to the next commitfest.

Committed, thanks.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 20/06/2016 06:28, Thomas Munro wrote:
> On Mon, Jun 20, 2016 at 3:43 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>> On 18 June 2016 at 11:28, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
>> wrote:
>>> Several times now when reading, debugging and writing code I've wished
>>> that LWLockHeldByMe assertions specified the expected mode, especially
>>> where exclusive locking is required.
>>>
>>> What do you think about something like the attached? See also an
>>> example of use. I will add this to the next commitfest.
>>
>> I've wanted this before too [...]
>

same here.

> Before ab5194e6f (25 December 2014) held_lwlocks didn't record the mode.
>

I just reviewed both patches. They applies cleanly on current HEAD,
work as intended and make check run smoothly. Patches are pretty
straightforward, so I don't have much to say.

My only remark is on following comment:

+ * LWLockHeldByMeInMode - test whether my process holds a lock in mode X

Maybe something like "test whether my process holds a lock in given
mode" would be better?

Otherwise, I think they're ready for committer.

--
Julien Rouhaud
http://dalibo.com - http://dalibo.org

On 05/09/2016 11:55, Julien Rouhaud wrote:
> On 20/06/2016 06:28, Thomas Munro wrote:
>> On Mon, Jun 20, 2016 at 3:43 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>>> On 18 June 2016 at 11:28, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
>>> wrote:
>>>> Several times now when reading, debugging and writing code I've wished
>>>> that LWLockHeldByMe assertions specified the expected mode, especially
>>>> where exclusive locking is required.
>>>>
>>>> What do you think about something like the attached? See also an
>>>> example of use. I will add this to the next commitfest.
>>>
>>> I've wanted this before too [...]
>>
>
> same here.
>
>> Before ab5194e6f (25 December 2014) held_lwlocks didn't record the mode.
>>
>
> I just reviewed both patches. They applies cleanly on current HEAD,
> work as intended and make check run smoothly. Patches are pretty
> straightforward, so I don't have much to say.
>
> My only remark is on following comment:
>
> + * LWLockHeldByMeInMode - test whether my process holds a lock in mode X
>
> Maybe something like "test whether my process holds a lock in given
> mode" would be better?
>
> Otherwise, I think they're ready for committer.
>

Didn't saw that Simon just committed it, sorry about it.

--
Julien Rouhaud
http://dalibo.com - http://dalibo.org