Thread: 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
Attachment
On 18 June 2016 at 11:28, Thomas Munro <thomas.munro@enterprisedb.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.
On Mon, Jun 20, 2016 at 3:43 PM, Craig Ringer <craig@2ndquadrant.com> wrote: > On 18 June 2016 at 11:28, Thomas Munro <thomas.munro@enterprisedb.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.munro@enterprisedb.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@2ndquadrant.com> wrote: >> On 18 June 2016 at 11:28, Thomas Munro <thomas.munro@enterprisedb.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@2ndquadrant.com> wrote: >>> On 18 June 2016 at 11:28, Thomas Munro <thomas.munro@enterprisedb.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