On Tue, Dec 12, 2017 at 5:15 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Sun, Dec 10, 2017 at 11:51 PM, Masahiko Sawada <sawada.mshk@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