> On Wednesday, 3 June 2015, 19:43, "fanghuang.data@xxxxxxxxx" <fanghuang.data@xxxxxxxxx> wrote: > > On Wednesday, 3 June 2015, 15:22, Xavier Hernandez <xhernandez@xxxxxxxxxx> > wrote: > >> On 06/03/2015 05:40 AM, Pranith Kumar Karampuri wrote: >>> On 06/02/2015 08:08 PM, fanghuang.data@xxxxxxxxx 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 _______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://www.gluster.org/mailman/listinfo/gluster-devel
