On Mon, Dec 21, 2015 at 1:27 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Fri, Dec 18, 2015 at 9:58 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>
>> On Fri, Dec 18, 2015 at 1:16 AM, Amit Kapila <amit.kapila16@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. Otherwise it might be
confused with some other time of grouping of PGPROCs.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company