Re: [PATCH] LockAcquireExtended improvement - Mailing list pgsql-hackers

From Robert Haas
Subject Re: [PATCH] LockAcquireExtended improvement
Date
Msg-id CA+TgmoaYwx7iCfB88xRykMTTORA+1MYkczzmykt4=W=DYTyKMw@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] LockAcquireExtended improvement  ("Jingxian Li" <aqktjcm@qq.com>)
List pgsql-hackers
On Thu, Feb 1, 2024 at 2:16 AM Jingxian Li <aqktjcm@qq.com> wrote:
> According to what you said, I resubmitted a patch which splits the ProcSleep
> logic into two parts, the former is responsible for inserting self to
> WaitQueue,
> the latter is responsible for deadlock detection and processing, and the
> former part is directly called by LockAcquireExtended before nowait fails.
> In this way the nowait case can also benefit from adjusting the insertion
> order of WaitQueue.

I don't have time for a full review of this patch right now
unfortunately, but just looking at it quickly:

- It will be helpful if you write a clear commit message. If it gets
committed, there is a high chance the committer will rewrite your
message, but in the meantime it will help understanding.

- The comment for InsertSelfIntoWaitQueue needs improvement. It is
only one line. And it says "Insert self into queue if dontWait is
false" but then someone will wonder why the function would ever be
called with dontWait = true.

- Between the comments and the commit message, the division of
responsibilities between InsertSelfIntoWaitQueue and ProcSleep needs
to be clearly explained. Right now I don't think it is.

--
Robert Haas
EDB: http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Collation version tracking for macOS
Next
From: Kartyshov Ivan
Date:
Subject: Fwd: Re: [HACKERS] make async slave to wait for lsn to be replayed