From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Subject: | Re: [PATCH] Refactoring of LWLock tranches |
Date: | 2016-02-11 17:10:23 |
Message-ID: | CAA4eK1Lj+Aok8orhn-2xGzR8nN6ngZvJqqGgTzZs1+KNAYmevA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Feb 11, 2016 at 6:45 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Thu, Feb 11, 2016 at 3:15 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
> > Sounds sensible, however after changes, CreateLWLocks() started
> > looking unreadable, so moved initialization and registration of tranches
> > to separate functions.
>
> Seems good.
>
> > Increased number of tranches allocated in LWLockTrancheArray, as
> > now the LWTRANCHE_FIRST_USER_DEFINED is already greater
> > than 16.
>
> OK, cool.
>
> + int numIndividualLocks =
NUM_INDIVIDUAL_LWLOCKS;
> + int numBufMappingLocks =
NUM_BUFFER_PARTITIONS;
> + int numLmgrLocks = NUM_LOCK_PARTITIONS;
> + int i;
>
> This doesn't seem like it's buying us anything. Can't we just use the
> constants directly?
>
Yes, we can use the constants. The reason I have used variables
was they need to be used at couple of places, but I think using
constants directly also doesn't harm, so changed accordingly.
> + BufMappingLWLockTranche.name = "Buffer Mapping Locks";
>
> We've not been very consistent about how we name our tranches. If
> we're going to try to expose this information to users, we're going to
> need to do better. We've got these names:
>
> main
> clog
> commit_timestamp
> subtrans
> multixact_offset
> multixact_member
> async
> oldserxid
> WALInsertLocks
> Buffer Content Locks
> Buffer IO Locks
> ReplicationOrigins
> Replication Slot IO Locks
> proc
>
> Personally, I prefer the style of all lower case, words separated with
> semicolons where necessary, the word "locks" left out. That way we
> can display wait events as something like lwlock:<name of tranche> and
> not have the word "lock" in there twice. I don't think it does much
> good to try to make these names "user friendly" because,
> fundamentally, these are internal pieces of the system and you won't
> know what they are unless you do. So I'd prefer to make them look
> like identifiers. If we adopt that style, then I think the new
> tranches created by this patch should be named buffer_mapping,
> lock_manager, and predicate_lock_manager
Okay, changed as per suggestion.
> and then, probably as a
> separate patch, we should rename the rest to match that style
> (buffer_content, buffer_io, replication_origin, replication_slot_io).
>
Sure, will send these changes as a separate patch.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
fixed_locks_tranche_v3.patch | application/octet-stream | 7.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2016-02-11 17:11:26 | Re: Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution. |
Previous Message | Tom Lane | 2016-02-11 17:04:39 | Re: Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution. |