[Gluster-devel] answer_list in EC xlator

[fanghuang.data at yahoo.com]

> On Wednesday, 3 June 2015, 19:43, "fanghuang.data at yahoo.com" <fanghuang.data at yahoo.com> wrote:

> > On Wednesday, 3 June 2015, 15:22, Xavier Hernandez <xhernandez at datalab.es> 
> wrote:
> 
>> On 06/03/2015 05:40 AM, Pranith Kumar Karampuri wrote:
>>>  On 06/02/2015 08:08 PM, fanghuang.data at yahoo.com wrote:
>>>>  Hi all,
>>>> 
>>>>  As I reading the source codes of EC xlator, I am confused by the
>>>>  cbk_list and answer_list defined in struct _ec_fop_data. Why do we
>>>>  need two lists to combine the results of callback?
>>>> 
>>>>  Especially for the answer_list, it is initialized
>>>>  in ec_fop_data_allocate, then the nodes are added
>>>>  in ec_cbk_data_allocate. Without being any accessed during the
>>>>  lifetime of fop, the whole list finally is released in 
> ec_fop_cleanup.
>>>>  Do I miss something for the answer_list?
>>>  +Xavi.
>>> 
>>>  hi,
>>>       The only reason I found is that It is easier to cleanup cbks using
>>>  answers_list. You can check ec_fop_cleanup() function on latest master
>>>  to check how this is.
>> 
>> You are right. Currently answer_list is only used to cleanup all cbks 
>> received while cbk_list is used to track groups of consistent answers. 
>> Although currently doesn't happen, if error coercing or special 
>> attribute handling are implemented, it could be possible that one cbk 
>> gets referenced more than once in cbk_list, making answer_list 
>> absolutely necessary.
>> 
> 
> That's a good point to put all the cbks into one group and put those with
> consistent answers into the other group. But this designing policy cannot 
> be understood easily from the comments, source codes or the list names 
> (cbk_list,
> answer_list). Could we rename the cbk_list to consist_list or something else 
> easier to be followed?
> 
>> 
>>>  Combining of cbks is a bit involved until you
>>>  understand it but once you do, it is amazing. I tried to add comments
>>>  for this part of code and sent a patch, but we forgot to merge it :-)
>>>  http://review.gluster.org/9982. If you think we can add more
>>>  comments/change this part of code in a way it makes it easier, let us
>>>  know. We would love your feedback :-). Wait for Xavi's response as 
> well.
>>> 
> 
> This patch is much clearer. For the function ec_combine_update_groups,
> since we only operate on one list, should we use ec_combine_update_group? The
> word "groups" is confusing for readers who may think there are two or 
> more groups.
> 


I got it finally. The cbk-list actually maintains multi-groups of the same answer sorted
by the count. As Xavi said, one cbk may exist in different groups. So we need an
answer_list to do cleanup job. Pranith's patch explains it clearly. Well it is 
really amazing.

--
Fang Huang