[Gluster-devel] answer_list in EC xlator

[Pranith Kumar Karampuri]

On 06/03/2015 09:21 PM, fanghuang.data at yahoo.com wrote:
>> 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.
Told ya! :-). I will resend the patch with updated comments about how 
the groups work.

Pranith
>
> --
> Fang Huang