Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.80) (envelope-from ) id 1aTMBE-0005p1-B0 for pgsql-hackers@arkaria.postgresql.org; Wed, 10 Feb 2016 04:15:04 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84) (envelope-from ) id 1aTMBD-0008V3-HP for pgsql-hackers@arkaria.postgresql.org; Wed, 10 Feb 2016 04:15:03 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84) (envelope-from ) id 1aTMBB-0008Th-TD for pgsql-hackers@postgresql.org; Wed, 10 Feb 2016 04:15:01 +0000 Received: from mail-wm0-x229.google.com ([2a00:1450:400c:c09::229]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84) (envelope-from ) id 1aTMB4-00064W-BD for pgsql-hackers@postgresql.org; Wed, 10 Feb 2016 04:15:01 +0000 Received: by mail-wm0-x229.google.com with SMTP id c200so10731050wme.0 for ; Tue, 09 Feb 2016 20:14:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=s8xJ1WCwlkGM4jnXAcbZoBJ3pW+cY74gF5ZdRVsZOKk=; b=0zsWPuajGUG5Mf2BCfztfw1/FVZWuPeAKRbRqlOefBTKaQfs58mv1PZiGyur7MC9co JJLhe/b5x6KdrYv4yFYZqmhsfBMsZwoFNFFBaLOY3xYrAwBZMM5c0Y/J8JOBsF2xzXgd 9AdTpFlbbGogv6H7MrcACmj20bIiPshTsGfzRkHH0Yyp8puD/7FMqfXtGdRudsMdAWqY t/lKGrxjyv3NebF3hSjEu1EVe/1/Uz8LUSOI8btp7hMkjNWgnazRsM9yE8VXLAUrT4ub qwv5/00w5G9FRW7inIMNWa+xb7HoWwNQjFslVQ7eksSfN4bJClOtM9fyBCQYCvlTW+FT miSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=s8xJ1WCwlkGM4jnXAcbZoBJ3pW+cY74gF5ZdRVsZOKk=; b=ShiHrq73A2iL0k4AzkpoQWGRnu9KcPIBypAUEw93XQynjdsdB9gQWXm0xSbhvbg3qY o1s5+xGk2yq+4Z2AMklFs/5AXBVQCGenoAckXMQrvub5qHHxo1iam7vfnY972LkDsg26 IHNmTxWK2B00+zGC3V3pR+vo8zJVcf1Fl3tqflD/S5uHl+lqhOggoTd5lAmt/fQEQSvD P+j6QGQGpiaapIIDXM51MQZekIQIe0i4PT+EWOHzkBJlgviK647h9wa2wFMCalFdkjS7 emf7sN1jbqVagESNVgIL6yhOoFt6KA4wQPtgA9jA3JU8ZdwNljXQgSsPiSI8g3o8tmVj a6GQ== X-Gm-Message-State: AG10YOQSwQMPx9FeH4ztTp0rQcS8A3cv6oRVNByErIq2rYK1gfiaHzht0v5ztQo5peVIl3Gcnct5t/ty6V2Q2g== MIME-Version: 1.0 X-Received: by 10.28.97.136 with SMTP id v130mr8157921wmb.20.1455077693815; Tue, 09 Feb 2016 20:14:53 -0800 (PST) Received: by 10.28.28.5 with HTTP; Tue, 9 Feb 2016 20:14:53 -0800 (PST) In-Reply-To: References: <20150903114137.GE27649@awork2.anarazel.de> <55FC4C33.1050903@redhat.com> <55FFFE51.1060308@redhat.com> Date: Wed, 10 Feb 2016 09:44:53 +0530 Message-ID: Subject: Re: Speed up Clog Access by increasing CLOG buffers From: Amit Kapila To: Thom Brown Cc: Robert Haas , Simon Riggs , Jesper Pedersen , pgsql-hackers Content-Type: multipart/alternative; boundary=001a1148fa460f8524052b62adf0 X-Pg-Spam-Score: -2.4 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-hackers Precedence: bulk Sender: pgsql-hackers-owner@postgresql.org --001a1148fa460f8524052b62adf0 Content-Type: text/plain; charset=UTF-8 On Tue, Feb 9, 2016 at 7:26 PM, Thom Brown wrote: > > On 7 January 2016 at 05:24, Amit Kapila wrote: > > On Fri, Dec 25, 2015 at 6:36 AM, Robert Haas wrote: > >> > >> On Wed, Dec 23, 2015 at 1:16 AM, Amit Kapila > >> wrote: > >> >> > > >> >> > Here procArrayGroupXid sounds like Xid at group level, how about > >> >> > procArrayGroupMemberXid? > >> >> > Find the patch with renamed variables for PGProc > >> >> > (rename_pgproc_variables_v1.patch) attached with mail. > >> >> > >> >> I sort of hate to make these member names any longer, but I wonder if > >> >> we should make it procArrayGroupClearXid etc. > >> > > >> > If we go by this suggestion, then the name will look like: > >> > PGProc > >> > { > >> > .. > >> > bool procArrayGroupClearXid, pg_atomic_uint32 > >> > procArrayGroupNextClearXid, > >> > TransactionId procArrayGroupLatestXid; > >> > .. > >> > > >> > PROC_HDR > >> > { > >> > .. > >> > pg_atomic_uint32 procArrayGroupFirstClearXid; > >> > .. > >> > } > >> > > >> > I think whatever I sent in last patch were better. It seems to me it is > >> > better to add some comments before variable names, so that anybody > >> > referring them can understand better and I have added comments in > >> > attached patch rename_pgproc_variables_v2.patch to explain the same. > >> > >> Well, I don't know. Anybody else have an opinion? > >> > > > > It seems that either people don't have any opinion on this matter or they > > are okay with either of the naming conventions being discussed. I think > > specifying Member after procArrayGroup can help distinguishing which > > variables are specific to the whole group and which are specific to a > > particular member. I think that will be helpful for other places as well > > if we use this technique to improve performance. Let me know what > > you think about the same. > > > > I have verified that previous patches can be applied cleanly and passes > > make check-world. To avoid confusion, I am attaching the latest > > patches with this mail. > > Patches still apply 1 month later. > Thanks for verification! > > I don't really have an opinion on the variable naming. I guess they > only need making longer if there's going to be some confusion about > what they're for, makes sense, that is the reason why I have added few comments as well, but not sure if you are suggesting something else. > but I'm guessing it's not a blocker here. > I also think so, but not sure what else is required here. The basic idea of this rename_pgproc_variables_v2.patch is to rename few variables in existing similar code, so that the main patch group_update_clog can adapt those naming convention if required, other than that I have handled all review comments raised in this thread (mainly by Simon and Robert). Is there anything, I can do to move this forward? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com --001a1148fa460f8524052b62adf0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On Tue, Feb 9, 2016 at 7:26 PM, Thom Brown <thom@linux.com> wrote:
>
> On 7 J= anuary 2016 at 05:24, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Fri, Dec 25, 2= 015 at 6:36 AM, Robert Haas <ro= bertmhaas@gmail.com> wrote:
> >>
> >> On Wed= , Dec 23, 2015 at 1:16 AM, Amit Kapila <amit.kapila16@gmail.com>
> >> wrote:
> = >> >> >
> >> >> > Here procArrayGroupXi= d sounds like Xid at group level, how about
> >> >> > = procArrayGroupMemberXid?
> >> >> > Find the patch with= renamed variables for PGProc
> >> >> > (rename_pgproc= _variables_v1.patch) attached with mail.
> >> >>
> = >> >> I sort of hate to make these member names any longer, but= I wonder if
> >> >> we should make it procArrayGroupClea= rXid etc.
> >> >
> >> > If we go by this sugg= estion, then the name will look like:
> >> > PGProc
> = >> > {
> >> > ..
> >> > bool procArr= ayGroupClearXid, pg_atomic_uint32
> >> > procArrayGroupNextC= learXid,
> >> > TransactionId procArrayGroupLatestXid;
&g= t; >> > ..
> >> >
> >> > PROC_HDR> >> > {
> >> > ..
> >> > pg_ato= mic_uint32 procArrayGroupFirstClearXid;
> >> > ..
> &g= t;> > }
> >> >
> >> > I think whatever = I sent in last patch were better.=C2=A0 It seems to me it is
> >&g= t; > better to add some comments before variable names, so that anybody<= br>> >> > referring them can understand better and I have added= comments in
> >> > attached patch rename_pgproc_variables_v= 2.patch to explain the same.
> >>
> >> Well, I don&= #39;t know.=C2=A0 Anybody else have an opinion?
> >>
> &g= t;
> > It seems that either people don't have any opinion on t= his matter or they
> > are okay with either of the naming conventi= ons being discussed.=C2=A0 I think
> > specifying Member after pro= cArrayGroup can help distinguishing which
> > variables are specif= ic to the whole group and which are specific to a
> > particular m= ember.=C2=A0 I think that will be helpful for other places as well
> = > if we use this technique to improve performance.=C2=A0 Let me know wha= t
> > you think about the same.
> >
> > I have v= erified that previous patches can be applied cleanly and passes
> >= ; make check-world.=C2=A0 To avoid confusion, I am attaching the latest
= > > patches with this mail.
>
> Patches still apply 1 mon= th later.
>

Thanks for verification!
=C2=A0
>
>= I don't really have an opinion on the variable naming.=C2=A0 I guess t= hey
> only need making longer if there's going to be some confusi= on about
> what they're for,

makes = sense, that is the reason why I have added few comments
as well, but not sure if you are suggesting something else.
> but I'm guessing it's not a blocker here.
><= br>

I also think so, but not sure what else i= s required here.=C2=A0 The basic
idea=C2=A0of this=C2= =A0rename_pgproc_variables_v2.patch is to rename
few v= ariables in existing similar code, so that the main patch
group_update_clog can adapt those naming convention if required,
other than that I have handled all review comments raised = in this
thread (mainly by Simon and Robert).

Is there = anything, I can do to move this forward?


With Regards,
Amit Kapila= .
EnterpriseDB:=C2=A0http://www.enterprisedb.com
--001a1148fa460f8524052b62adf0--