On Thu, Feb 11, 2016 at 6:45 PM, Robert Haas <robertmh...@gmail.com> wrote: > > On Thu, Feb 11, 2016 at 3:15 AM, Amit Kapila <amit.kapil...@gmail.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
fixed_locks_tranche_v3.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers