Neighbors exclusion

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Neighbors exclusion

dkarachentsev
Hi folks,

Why RendezvousAffinityFunction.excludeNeighbors [1] is false by default?
It's not obvious that if user wants to run more than one node per
machine it has also set this flag to true explicitly. Maybe it would be
better to set it to true by default?

At the same time if excludeNeighbors is true, it ignores backupFilter.
Why it's not vice-versa? For example:

1) if backupFilter is set - it will be used,

2) if there are not enough backup nodes (or no backupFilter) - try to
distribute according to excludeNeighbors = true,

3) if this is not possible too (or excludeNeighbors) = false - assign
partitions as possible.

[1]
https://ignite.apache.org/releases/latest/javadoc/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.html#setExcludeNeighbors-boolean-

Are there any drawbacks in such approach?

Thanks!

Reply | Threaded
Open this post in threaded view
|

Re: Neighbors exclusion

Valentin Kulichenko
Dmitry,

Good point. I think it makes sense to even remove (deprecate) the
excludeNeighbors property and always distribute primary and backups to
different physical hosts in this scenario. Because why would anyone ever
set this to false if we switch default to true? This also automatically
fixes the confusing behavior of backupFilter - it should never be ignored
if it's set.

-Val

On Fri, Jul 13, 2018 at 8:05 AM Dmitry Karachentsev <
[hidden email]> wrote:

> Hi folks,
>
> Why RendezvousAffinityFunction.excludeNeighbors [1] is false by default?
> It's not obvious that if user wants to run more than one node per
> machine it has also set this flag to true explicitly. Maybe it would be
> better to set it to true by default?
>
> At the same time if excludeNeighbors is true, it ignores backupFilter.
> Why it's not vice-versa? For example:
>
> 1) if backupFilter is set - it will be used,
>
> 2) if there are not enough backup nodes (or no backupFilter) - try to
> distribute according to excludeNeighbors = true,
>
> 3) if this is not possible too (or excludeNeighbors) = false - assign
> partitions as possible.
>
> [1]
>
> https://ignite.apache.org/releases/latest/javadoc/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.html#setExcludeNeighbors-boolean-
>
> Are there any drawbacks in such approach?
>
> Thanks!
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Neighbors exclusion

dkarachentsev
Created a ticket and mapped to 3.0 version, as it changes basic default
behavior:
https://issues.apache.org/jira/browse/IGNITE-9011

Thanks!

13.07.2018 22:10, Valentin Kulichenko пишет:

> Dmitry,
>
> Good point. I think it makes sense to even remove (deprecate) the
> excludeNeighbors property and always distribute primary and backups to
> different physical hosts in this scenario. Because why would anyone ever
> set this to false if we switch default to true? This also automatically
> fixes the confusing behavior of backupFilter - it should never be ignored
> if it's set.
>
> -Val
>
> On Fri, Jul 13, 2018 at 8:05 AM Dmitry Karachentsev <
> [hidden email]> wrote:
>
>> Hi folks,
>>
>> Why RendezvousAffinityFunction.excludeNeighbors [1] is false by default?
>> It's not obvious that if user wants to run more than one node per
>> machine it has also set this flag to true explicitly. Maybe it would be
>> better to set it to true by default?
>>
>> At the same time if excludeNeighbors is true, it ignores backupFilter.
>> Why it's not vice-versa? For example:
>>
>> 1) if backupFilter is set - it will be used,
>>
>> 2) if there are not enough backup nodes (or no backupFilter) - try to
>> distribute according to excludeNeighbors = true,
>>
>> 3) if this is not possible too (or excludeNeighbors) = false - assign
>> partitions as possible.
>>
>> [1]
>>
>> https://ignite.apache.org/releases/latest/javadoc/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.html#setExcludeNeighbors-boolean-
>>
>> Are there any drawbacks in such approach?
>>
>> Thanks!
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: Neighbors exclusion

yzhdanov
Dmitry, it hink we can do this change right away. All we need is to add
proper error message on cache config validation in order to tell user that
default changed and manual configuration is needed for compatibility.

--Yakov

2018-07-16 15:47 GMT+03:00 Dmitry Karachentsev <[hidden email]>:

> Created a ticket and mapped to 3.0 version, as it changes basic default
> behavior:
> https://issues.apache.org/jira/browse/IGNITE-9011
>
> Thanks!
>
> 13.07.2018 22:10, Valentin Kulichenko пишет:
>
> Dmitry,
>>
>> Good point. I think it makes sense to even remove (deprecate) the
>> excludeNeighbors property and always distribute primary and backups to
>> different physical hosts in this scenario. Because why would anyone ever
>> set this to false if we switch default to true? This also automatically
>> fixes the confusing behavior of backupFilter - it should never be ignored
>> if it's set.
>>
>> -Val
>>
>> On Fri, Jul 13, 2018 at 8:05 AM Dmitry Karachentsev <
>> [hidden email]> wrote:
>>
>> Hi folks,
>>>
>>> Why RendezvousAffinityFunction.excludeNeighbors [1] is false by default?
>>> It's not obvious that if user wants to run more than one node per
>>> machine it has also set this flag to true explicitly. Maybe it would be
>>> better to set it to true by default?
>>>
>>> At the same time if excludeNeighbors is true, it ignores backupFilter.
>>> Why it's not vice-versa? For example:
>>>
>>> 1) if backupFilter is set - it will be used,
>>>
>>> 2) if there are not enough backup nodes (or no backupFilter) - try to
>>> distribute according to excludeNeighbors = true,
>>>
>>> 3) if this is not possible too (or excludeNeighbors) = false - assign
>>> partitions as possible.
>>>
>>> [1]
>>>
>>> https://ignite.apache.org/releases/latest/javadoc/org/apache
>>> /ignite/cache/affinity/rendezvous/RendezvousAffinityFunction
>>> .html#setExcludeNeighbors-boolean-
>>>
>>> Are there any drawbacks in such approach?
>>>
>>> Thanks!
>>>
>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Neighbors exclusion

slava.koptilin
In reply to this post by dkarachentsev
Hi Dmitry,

I agree with Val. It seems to me that this flag does not make sense and we
can get rid of it.

The most important thing that you mentioned (at least for me) is the
following:
> 2. if there are not enough backup nodes (or no backupFilter) - try to
distribute according to excludeNeighbors = true
The current implementation of RendezvousAffinity function just silently
ignore this case and does not attempt to assign partitions on other nodes.
I mean the case when backupFilter is used. We definitely need to try to
assign partitions on other nodes even if that nodes do not satisfy the
backupFilter.
And, of course, a proper message should be printed in a log file.

Thanks,
Slava.


пт, 13 июл. 2018 г. в 18:05, Dmitry Karachentsev <[hidden email]
>:

> Hi folks,
>
> Why RendezvousAffinityFunction.excludeNeighbors [1] is false by default?
> It's not obvious that if user wants to run more than one node per
> machine it has also set this flag to true explicitly. Maybe it would be
> better to set it to true by default?
>
> At the same time if excludeNeighbors is true, it ignores backupFilter.
> Why it's not vice-versa? For example:
>
> 1) if backupFilter is set - it will be used,
>
> 2) if there are not enough backup nodes (or no backupFilter) - try to
> distribute according to excludeNeighbors = true,
>
> 3) if this is not possible too (or excludeNeighbors) = false - assign
> partitions as possible.
>
> [1]
>
> https://ignite.apache.org/releases/latest/javadoc/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.html#setExcludeNeighbors-boolean-
>
> Are there any drawbacks in such approach?
>
> Thanks!
>
>