Re: [Gluster-devel] Should we enable contention notification by default ?

2019-06-06 Thread Xavi Hernandez
Missed the patch link: https://review.gluster.org/c/glusterfs/+/22828

On Thu, Jun 6, 2019 at 8:32 AM Xavi Hernandez  wrote:

> On Thu, May 2, 2019 at 5:45 PM Atin Mukherjee 
> wrote:
>
>>
>>
>> On Thu, 2 May 2019 at 20:38, Xavi Hernandez 
>> 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 
 wrote:

> On Thu, 2 May 2019, 15:37 Milind Changire, 
> wrote:
>
>> On Thu, May 2, 2019 at 6:44 PM Xavi Hernandez 
>> wrote:
>>
>>> Hi Ashish,
>>>
>>> On Thu, May 2, 2019 at 2:17 PM Ashish Pandey 
>>> 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" 
 *To: *"gluster-devel" 
 *Cc: *"Pranith Kumar Karampuri" , "Ashish
 Pandey" , "Amar Tumballi" >>> >
 *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, 

Re: [Gluster-devel] Should we enable contention notification by default ?

2019-06-06 Thread Xavi Hernandez
On Thu, May 2, 2019 at 5:45 PM Atin Mukherjee 
wrote:

>
>
> On Thu, 2 May 2019 at 20:38, Xavi Hernandez  wrote:
>
>> On Thu, May 2, 2019 at 4:06 PM Atin Mukherjee 
>> wrote:
>>
>>>
>>>
>>> On Thu, 2 May 2019 at 19:14, Xavi Hernandez 
>>> wrote:
>>>
 On Thu, 2 May 2019, 15:37 Milind Changire,  wrote:

> On Thu, May 2, 2019 at 6:44 PM Xavi Hernandez 
> wrote:
>
>> Hi Ashish,
>>
>> On Thu, May 2, 2019 at 2:17 PM Ashish Pandey 
>> 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" 
>>> *To: *"gluster-devel" 
>>> *Cc: *"Pranith Kumar Karampuri" , "Ashish
>>> Pandey" , "Amar Tumballi" 
>>> *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 

Re: [Gluster-devel] Should we enable contention notification by default ?

2019-05-02 Thread Atin Mukherjee
On Thu, 2 May 2019 at 20:38, Xavi Hernandez  wrote:

> On Thu, May 2, 2019 at 4:06 PM Atin Mukherjee 
> wrote:
>
>>
>>
>> On Thu, 2 May 2019 at 19:14, Xavi Hernandez 
>> wrote:
>>
>>> On Thu, 2 May 2019, 15:37 Milind Changire,  wrote:
>>>
 On Thu, May 2, 2019 at 6:44 PM Xavi Hernandez 
 wrote:

> Hi Ashish,
>
> On Thu, May 2, 2019 at 2:17 PM Ashish Pandey 
> 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" 
>> *To: *"gluster-devel" 
>> *Cc: *"Pranith Kumar Karampuri" , "Ashish
>> Pandey" , "Amar Tumballi" 
>> *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
> 

Re: [Gluster-devel] Should we enable contention notification by default ?

2019-05-02 Thread Xavi Hernandez
On Thu, May 2, 2019 at 4:06 PM Atin Mukherjee 
wrote:

>
>
> On Thu, 2 May 2019 at 19:14, Xavi Hernandez  wrote:
>
>> On Thu, 2 May 2019, 15:37 Milind Changire,  wrote:
>>
>>> On Thu, May 2, 2019 at 6:44 PM Xavi Hernandez 
>>> wrote:
>>>
 Hi Ashish,

 On Thu, May 2, 2019 at 2:17 PM Ashish Pandey 
 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" 
> *To: *"gluster-devel" 
> *Cc: *"Pranith Kumar Karampuri" , "Ashish
> Pandey" , "Amar Tumballi" 
> *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 ?


>
> 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
>
___

Re: [Gluster-devel] Should we enable contention notification by default ?

2019-05-02 Thread Atin Mukherjee
On Thu, 2 May 2019 at 19:14, Xavi Hernandez  wrote:

> On Thu, 2 May 2019, 15:37 Milind Changire,  wrote:
>
>> On Thu, May 2, 2019 at 6:44 PM Xavi Hernandez 
>> wrote:
>>
>>> Hi Ashish,
>>>
>>> On Thu, May 2, 2019 at 2:17 PM Ashish Pandey 
>>> 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" 
 *To: *"gluster-devel" 
 *Cc: *"Pranith Kumar Karampuri" , "Ashish Pandey"
 , "Amar Tumballi" 
 *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.


 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
___
Gluster-devel mailing list
Gluster-devel@gluster.org
https://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] Should we enable contention notification by default ?

2019-05-02 Thread Xavi Hernandez
On Thu, 2 May 2019, 15:37 Milind Changire,  wrote:

> On Thu, May 2, 2019 at 6:44 PM Xavi Hernandez 
> wrote:
>
>> Hi Ashish,
>>
>> On Thu, May 2, 2019 at 2:17 PM Ashish Pandey  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" 
>>> *To: *"gluster-devel" 
>>> *Cc: *"Pranith Kumar Karampuri" , "Ashish Pandey" <
>>> aspan...@redhat.com>, "Amar Tumballi" 
>>> *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 ?
>>>
>>> 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

Re: [Gluster-devel] Should we enable contention notification by default ?

2019-05-02 Thread Milind Changire
On Thu, May 2, 2019 at 6:44 PM Xavi Hernandez  wrote:

> Hi Ashish,
>
> On Thu, May 2, 2019 at 2:17 PM Ashish Pandey  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.


>  .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" 
>> *To: *"gluster-devel" 
>> *Cc: *"Pranith Kumar Karampuri" , "Ashish Pandey" <
>> aspan...@redhat.com>, "Amar Tumballi" 
>> *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 ?
>>
>> 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

Re: [Gluster-devel] Should we enable contention notification by default ?

2019-05-02 Thread Xavi Hernandez
Hi Ashish,

On Thu, May 2, 2019 at 2:17 PM Ashish Pandey  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"
>  .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" 
> *To: *"gluster-devel" 
> *Cc: *"Pranith Kumar Karampuri" , "Ashish Pandey" <
> aspan...@redhat.com>, "Amar Tumballi" 
> *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 ?
>
> Regards,
>
> Xavi
>
>
___
Gluster-devel mailing list
Gluster-devel@gluster.org
https://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] Should we enable contention notification by default ?

2019-05-02 Thread Ashish Pandey
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" 
.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. 
Is my understanding correct? 
What will be impact of this value and what should be the default value of this 
option? 

--- 
Ashish 






- Original Message -

From: "Xavi Hernandez"  
To: "gluster-devel"  
Cc: "Pranith Kumar Karampuri" , "Ashish Pandey" 
, "Amar Tumballi"  
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 ? 

Regards, 

Xavi 

___
Gluster-devel mailing list
Gluster-devel@gluster.org
https://lists.gluster.org/mailman/listinfo/gluster-devel