Re: [Gluster-devel] answer_list in EC xlator
On 06/03/2015 09:21 PM, fanghuang.d...@yahoo.com wrote: On Wednesday, 3 June 2015, 19:43, "fanghuang.d...@yahoo.com" wrote: On Wednesday, 3 June 2015, 15:22, Xavier Hernandez wrote: On 06/03/2015 05:40 AM, Pranith Kumar Karampuri wrote: On 06/02/2015 08:08 PM, fanghuang.d...@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 ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] answer_list in EC xlator
> On Wednesday, 3 June 2015, 19:43, "fanghuang.d...@yahoo.com" > wrote: > > On Wednesday, 3 June 2015, 15:22, Xavier Hernandez > wrote: > >> On 06/03/2015 05:40 AM, Pranith Kumar Karampuri wrote: >>> On 06/02/2015 08:08 PM, fanghuang.d...@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 ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] answer_list in EC xlator
On Wednesday, 3 June 2015, 15:22, Xavier Hernandez wrote: > > >Hi, > >On 06/03/2015 05:40 AM, Pranith Kumar Karampuri wrote: >> >> >> On 06/02/2015 08:08 PM, fanghuang.d...@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. -- Fang Huang ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] answer_list in EC xlator
Hi, On 06/03/2015 05:40 AM, Pranith Kumar Karampuri wrote: On 06/02/2015 08:08 PM, fanghuang.d...@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. 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. Pranith Regards, Fang Huang ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] answer_list in EC xlator
On 06/02/2015 08:08 PM, fanghuang.d...@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. 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. Pranith Regards, Fang Huang ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
[Gluster-devel] answer_list in EC xlator
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? Regards, Fang Huang___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
