On Tue, Dec 12, 2017 at 5:15 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Sun, Dec 10, 2017 at 11:51 PM, Masahiko Sawada <sawada.m...@gmail.com> > wrote: >> Attached updated version patch. Please review it. > > I went over this today; please find attached an updated version which > I propose to commit. > > Changes: > > - Various formatting fixes, including running pgindent. > > - Various comment updates. > > - Make RELEXT_WAIT_COUNT_MASK equal RELEXT_LOCK_BIT - 1 rather than > some unnecessarily smaller number. > > - In InitRelExtLocks, don't bother using mul_size; we already know it > won't overflow, because we did the same thing in RelExtLockShmemSize. > > - When we run into an error trying to release a lock, log it as a > WARNING and don't mark it as translatable. Follows lock.c. An ERROR > here probably just recurses infinitely. > > - Don't bother passing OID to RelExtLockRelease. > > - Reorder functions a bit for (IMHO) better clarity. > > - Make UnlockRelationForExtension just use a single message for both > failure modes. They are closely-enough related that I think that's > fine. > > - Make WaitForRelationExtensionLockToBeFree complain if we already > hold an extension lock. > > - In RelExtLockCleanup, clear held_relextlock.waiting. This would've > made for a nasty bug. > > - Also in that function, assert that we don't hold both a lock and a wait > count. >
Thank you for updating the patch. Here is two minor comments. + * we acquire the same relation extension lock repeatedly. nLocks is 0 is the + * number of times we've acquired that lock; Should it be "nLocks is the number of times we've acquired that lock:"? + /* Remember lock held by this backend */ + held_relextlock.relid = relid; + held_relextlock.lock = relextlock; + held_relextlock.nLocks = 1; We set held_relextlock.relid and held_relextlock.lock again. Can we remove them? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center