On Wed, Dec 23, 2015 at 1:16 AM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Tue, Dec 22, 2015 at 10:43 PM, Robert Haas <robertmh...@gmail.com> wrote:
>>
>> On Mon, Dec 21, 2015 at 1:27 AM, Amit Kapila <amit.kapil...@gmail.com>
>> wrote:
>> > On Fri, Dec 18, 2015 at 9:58 PM, Robert Haas <robertmh...@gmail.com>
>> > wrote:
>> >>
>> >> On Fri, Dec 18, 2015 at 1:16 AM, Amit Kapila <amit.kapil...@gmail.com>
>> >> wrote:
>> >>
>> >> >> Some random comments:
>> >> >>
>> >> >> - TransactionGroupUpdateXidStatus could do just as well without
>> >> >> add_proc_to_group. You could just say if (group_no >= NUM_GROUPS)
>> >> >> break; instead. Also, I think you could combine the two if
>> >> >> statements
>> >> >> inside the loop. if (nextidx != INVALID_PGPROCNO &&
>> >> >> ProcGlobal->allProcs[nextidx].clogPage == proc->clogPage) break; or
>> >> >> something like that.
>> >> >>
>> >
>> > Changed as per suggestion.
>> >
>> >> >> - memberXid and memberXidstatus are terrible names. Member of what?
>> >> >
>> >> > How about changing them to clogGroupMemberXid and
>> >> > clogGroupMemberXidStatus?
>> >>
>> >> What we've currently got for group XID clearing for the ProcArray is
>> >> clearXid, nextClearXidElem, and backendLatestXid. We should try to
>> >> make these things consistent. Maybe rename those to
>> >> procArrayGroupMember, procArrayGroupNext, procArrayGroupXid
>> >>
>> >
>> > 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?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
