On Thu, 2 May 2019 at 20:38, Xavi Hernandez <xhernan...@redhat.com> wrote:
> On Thu, May 2, 2019 at 4:06 PM Atin Mukherjee <atin.mukherje...@gmail.com> > wrote: > >> >> >> On Thu, 2 May 2019 at 19:14, Xavi Hernandez <xhernan...@redhat.com> >> wrote: >> >>> On Thu, 2 May 2019, 15:37 Milind Changire, <mchan...@redhat.com> wrote: >>> >>>> On Thu, May 2, 2019 at 6:44 PM Xavi Hernandez <xhernan...@redhat.com> >>>> wrote: >>>> >>>>> Hi Ashish, >>>>> >>>>> On Thu, May 2, 2019 at 2:17 PM Ashish Pandey <aspan...@redhat.com> >>>>> wrote: >>>>> >>>>>> Xavi, >>>>>> >>>>>> I would like to keep this option (features.lock-notify-contention) >>>>>> enabled by default. >>>>>> However, I can see that there is one more option which will impact >>>>>> the working of this option which is "notify-contention-delay" >>>>>> >>>>> >>>> Just a nit. I wish the option was called "notify-contention-interval" >>>> The "delay" part doesn't really emphasize where the delay would be put >>>> in. >>>> >>> >>> It makes sense. Maybe we can also rename it or add a second name >>> (alias). If there are no objections, I will send a patch with the change. >>> >>> Xavi >>> >>> >>>> >>>>> .description = "This value determines the minimum amount of time " >>>>>> "(in seconds) between upcall contention >>>>>> notifications " >>>>>> "on the same inode. If multiple lock requests are >>>>>> " >>>>>> "received during this period, only one upcall >>>>>> will " >>>>>> "be sent."}, >>>>>> >>>>>> I am not sure what should be the best value for this option if we >>>>>> want to keep features.lock-notify-contention ON by default? >>>>>> It looks like if we keep the value of notify-contention-delay more, >>>>>> say 5 sec, it will wait for this much time to send up call >>>>>> notification which does not look good. >>>>>> >>>>> >>>>> No, the first notification is sent immediately. What this option does >>>>> is to define the minimum interval between notifications. This interval is >>>>> per lock. This is done to avoid storms of notifications if many requests >>>>> come referencing the same lock. >>>>> >>>>> Is my understanding correct? >>>>>> What will be impact of this value and what should be the default >>>>>> value of this option? >>>>>> >>>>> >>>>> I think the current default value of 5 seconds seems good enough. If >>>>> there are many bricks, each brick could send a notification per lock. 1000 >>>>> bricks would mean a client would receive 1000 notifications every 5 >>>>> seconds. It doesn't seem too much, but in those cases 10, and considering >>>>> we could have other locks, maybe a higher value could be better. >>>>> >>>>> Xavi >>>>> >>>>> >>>>>> >>>>>> --- >>>>>> Ashish >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> ------------------------------ >>>>>> *From: *"Xavi Hernandez" <xhernan...@redhat.com> >>>>>> *To: *"gluster-devel" <gluster-devel@gluster.org> >>>>>> *Cc: *"Pranith Kumar Karampuri" <pkara...@redhat.com>, "Ashish >>>>>> Pandey" <aspan...@redhat.com>, "Amar Tumballi" <atumb...@redhat.com> >>>>>> *Sent: *Thursday, May 2, 2019 4:15:38 PM >>>>>> *Subject: *Should we enable contention notification by default ? >>>>>> >>>>>> Hi all, >>>>>> >>>>>> there's a feature in the locks xlator that sends a notification to >>>>>> current owner of a lock when another client tries to acquire the same >>>>>> lock. >>>>>> This way the current owner is made aware of the contention and can >>>>>> release >>>>>> the lock as soon as possible to allow the other client to proceed. >>>>>> >>>>>> This is specially useful when eager-locking is used and multiple >>>>>> clients access the same files and directories. Currently both replicated >>>>>> and dispersed volumes use eager-locking and can use contention >>>>>> notification >>>>>> to force an early release of the lock. >>>>>> >>>>>> Eager-locking reduces the number of network requests required for >>>>>> each operation, improving performance, but could add delays to other >>>>>> clients while it keeps the inode or entry locked. With the contention >>>>>> notification feature we avoid this delay, so we get the best performance >>>>>> with minimal issues in multiclient environments. >>>>>> >>>>>> Currently the contention notification feature is controlled by the >>>>>> 'features.lock-notify-contention' option and it's disabled by default. >>>>>> Should we enable it by default ? >>>>>> >>>>>> I don't see any reason to keep it disabled by default. Does anyone >>>>>> foresee any problem ? >>>>>> >>>>> >> Is it a server only option? Otherwise it will break backward >> compatibility if we rename the key, If alias can get this fixed, that’s a >> better choice but I’m not sure if it solves all the problems. >> > > It's a server side option. I though that an alias didn't have any other > implication than accept two names for the same option. Is there anything > else I need to consider ? > If it’s a server side option then there’s no challenge in alias. If you do rename then in heterogeneous server versions volume set wouldn’t work though. > >> >>>>>> Regards, >>>>>> >>>>>> Xavi >>>>>> >>>>>> _______________________________________________ >>>>> Gluster-devel mailing list >>>>> Gluster-devel@gluster.org >>>>> https://lists.gluster.org/mailman/listinfo/gluster-devel >>>> >>>> >>>> >>>> -- >>>> Milind >>>> >>>> _______________________________________________ >>> Gluster-devel mailing list >>> Gluster-devel@gluster.org >>> https://lists.gluster.org/mailman/listinfo/gluster-devel >> >> -- >> --Atin >> > -- --Atin
_______________________________________________ Gluster-devel mailing list Gluster-devel@gluster.org https://lists.gluster.org/mailman/listinfo/gluster-devel