[Gluster-devel] answer_list in EC xlator
Pranith Kumar Karampuri
pkarampu at redhat.com
Thu Jun 4 01:52:17 UTC 2015
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
More information about the Gluster-devel
mailing list