Re: [Gluster-devel] conflicting keys for option eager-lock
On 04/13/2016 05:10 PM, Vijay Bellur wrote: On 04/13/2016 03:20 AM, Pranith Kumar Karampuri wrote: On 04/13/2016 11:58 AM, Pranith Kumar Karampuri wrote: On 04/13/2016 09:15 AM, Vijay Bellur wrote: On 04/08/2016 10:25 PM, Vijay Bellur wrote: Hey Pranith, Ashish - We have broken support for group virt after the following commit in release-3.7: commit 46920e3bd38d9ae7c1910d0bd83eff309ab20c66 Author: Ashish PandeyDate: Fri Mar 4 13:05:09 2016 +0530 cluster/ec: Provide an option to enable/disable eager lock Thinking more - do we need two different options to control eager lock behavior for afr and ec? cluster.eager-lock can be applicable for ec too as ec and afr are normally used in a mutually exclusive manner. Are we resorting to a different key for glusterd's op-version compatibility? Tiering breaks all our assumptions, at the moment we want afr to use eager-lock where as disperse to not for tiering, so it is better this way. This is only till we fix the eager-locking behavior in disperse in some cases where it is not learning that other clients are competing for locks. We already have a solution in place. After which both can be enabled for things to work smooth. Thanks, Pranith. In the interim can we also please fix the volume set help description for disperse.eager-lock? Yes, that will be done. Pranith -Vijay ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] conflicting keys for option eager-lock
On 04/13/2016 03:20 AM, Pranith Kumar Karampuri wrote: On 04/13/2016 11:58 AM, Pranith Kumar Karampuri wrote: On 04/13/2016 09:15 AM, Vijay Bellur wrote: On 04/08/2016 10:25 PM, Vijay Bellur wrote: Hey Pranith, Ashish - We have broken support for group virt after the following commit in release-3.7: commit 46920e3bd38d9ae7c1910d0bd83eff309ab20c66 Author: Ashish PandeyDate: Fri Mar 4 13:05:09 2016 +0530 cluster/ec: Provide an option to enable/disable eager lock Thinking more - do we need two different options to control eager lock behavior for afr and ec? cluster.eager-lock can be applicable for ec too as ec and afr are normally used in a mutually exclusive manner. Are we resorting to a different key for glusterd's op-version compatibility? Tiering breaks all our assumptions, at the moment we want afr to use eager-lock where as disperse to not for tiering, so it is better this way. This is only till we fix the eager-locking behavior in disperse in some cases where it is not learning that other clients are competing for locks. We already have a solution in place. After which both can be enabled for things to work smooth. Thanks, Pranith. In the interim can we also please fix the volume set help description for disperse.eager-lock? -Vijay ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] conflicting keys for option eager-lock
On 04/13/2016 11:58 AM, Pranith Kumar Karampuri wrote: On 04/13/2016 09:15 AM, Vijay Bellur wrote: On 04/08/2016 10:25 PM, Vijay Bellur wrote: Hey Pranith, Ashish - We have broken support for group virt after the following commit in release-3.7: commit 46920e3bd38d9ae7c1910d0bd83eff309ab20c66 Author: Ashish PandeyDate: Fri Mar 4 13:05:09 2016 +0530 cluster/ec: Provide an option to enable/disable eager lock Thinking more - do we need two different options to control eager lock behavior for afr and ec? cluster.eager-lock can be applicable for ec too as ec and afr are normally used in a mutually exclusive manner. Are we resorting to a different key for glusterd's op-version compatibility? Tiering breaks all our assumptions, at the moment we want afr to use eager-lock where as disperse to not for tiering, so it is better this way. This is only till we fix the eager-locking behavior in disperse in some cases where it is not learning that other clients are competing for locks. We already have a solution in place. After which both can be enabled for things to work smooth. Pranith Pranith Thanks, Vijay ___ 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] conflicting keys for option eager-lock
On 04/12/2016 11:53 PM, Atin Mukherjee wrote: On 04/13/2016 09:15 AM, Vijay Bellur wrote: On 04/08/2016 10:25 PM, Vijay Bellur wrote: Hey Pranith, Ashish - We have broken support for group virt after the following commit in release-3.7: commit 46920e3bd38d9ae7c1910d0bd83eff309ab20c66 Author: Ashish PandeyDate: Fri Mar 4 13:05:09 2016 +0530 cluster/ec: Provide an option to enable/disable eager lock Thinking more - do we need two different options to control eager lock behavior for afr and ec? cluster.eager-lock can be applicable for ec too as ec and afr are normally used in a mutually exclusive manner. Are we resorting to a different key for glusterd's op-version compatibility? I strongly recommend to use a different key as fixing them later is pain and we still don't have a solution to fix some of the existing options which are same across multiple xlators. From an usability perspective, having too many similar sounding options or too many options in general is a nightmare. Technical correctness and usability need to go together else the result would be chaotic. Looking at this very specific case, we now have two options "cluster.eager-lock" and "disperse.eager-lock". Several questions come to my mind when we look at the keys from a developer perspective: 1. What does cluster imply? Would I need to enable that to enable eager-lock for disperse volumes? 2. Since disperse is also a cluster translator, why do we have two different keys? If we look at output of volume set help: Option: cluster.eager-lock Default Value: on Description: Lock phase of a transaction has two sub-phases. First is an attempt to acquire locks in parallel by broadcasting non-blocking lock requests. If lock acquisition fails on any server, then the held locks are unlocked and revert to a blocking locked mode sequentially on one server after another. If this option is enabled the initial broadcasting lock request attempt to acquire lock on the entire file. If this fails, we revert back to the sequential "regional" blocking lock as before. In the case where such an "eager" lock is granted in the non-blocking phase, it gives rise to an opportunity for optimization. i.e, if the next write transaction on the same FD arrives before the unlock phase of the first transaction, it "takes over" the full file lock. Similarly if yet another data transaction arrives before the unlock phase of the "optimized" transaction, that in turn "takes over" the lock as well. The actual unlock now happens at the end of the last "optimized" transaction. Option: disperse.eager-lock Default Value: on Description: This option will enable/diable eager lock fordisperse volume cluster.eager-lock nowhere conveys that it is for replicate/afr volumes, disperse.eager-lock's description does contain few typos. With such descriptions confusions stemming from an user perspective would be even worse than what we perceive as developers .Usually when our behavior is not intuitive or confusing, that is when configuration errors happen. If we really need to maintain two different keys for correctness, we need to have the option keys,description convey more appropriate and relevant information for users at the least. Regards, Vijay ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] conflicting keys for option eager-lock
On 04/13/2016 09:15 AM, Vijay Bellur wrote: > On 04/08/2016 10:25 PM, Vijay Bellur wrote: >> Hey Pranith, Ashish - >> >> We have broken support for group virt after the following commit in >> release-3.7: >> >> commit 46920e3bd38d9ae7c1910d0bd83eff309ab20c66 >> Author: Ashish Pandey>> Date: Fri Mar 4 13:05:09 2016 +0530 >> >> cluster/ec: Provide an option to enable/disable eager lock >> >> > > Thinking more - do we need two different options to control eager lock > behavior for afr and ec? cluster.eager-lock can be applicable for ec too > as ec and afr are normally used in a mutually exclusive manner. Are we > resorting to a different key for glusterd's op-version compatibility? I strongly recommend to use a different key as fixing them later is pain and we still don't have a solution to fix some of the existing options which are same across multiple xlators. > > Thanks, > Vijay > ___ > 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] conflicting keys for option eager-lock
On 04/08/2016 10:25 PM, Vijay Bellur wrote: Hey Pranith, Ashish - We have broken support for group virt after the following commit in release-3.7: commit 46920e3bd38d9ae7c1910d0bd83eff309ab20c66 Author: Ashish PandeyDate: Fri Mar 4 13:05:09 2016 +0530 cluster/ec: Provide an option to enable/disable eager lock Thinking more - do we need two different options to control eager lock behavior for afr and ec? cluster.eager-lock can be applicable for ec too as ec and afr are normally used in a mutually exclusive manner. Are we resorting to a different key for glusterd's op-version compatibility? Thanks, Vijay ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel
[Gluster-devel] conflicting keys for option eager-lock
Hey Pranith, Ashish - We have broken support for group virt after the following commit in release-3.7: commit 46920e3bd38d9ae7c1910d0bd83eff309ab20c66 Author: Ashish PandeyDate: Fri Mar 4 13:05:09 2016 +0530 cluster/ec: Provide an option to enable/disable eager lock I have sent a patch to address this on master [1]. We would need to backport this patch to release-3.7 once merged on master and include it in 3.7.11. Kaushal - please hold tagging of 3.7.11 as there is an undesirable impact on oVirt & OpenStack workflows if we don't include this patch. Thanks, Vijay [1] http://review.gluster.org/#/c/13929 ___ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel