On Thu, Feb 1, 2024 at 2:16 AM Jingxian Li <aqkt...@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
