[infinispan-dev] Switch to AffinityPartitioner by default, or even enforce it

[Sanne Grinovero]

On 19 August 2016 at 15:39, Dan Berindei <dan.berindei at gmail.com> wrote:
> I just had a discussion with Adrian, and he reminded me that the key
> partitioners don't yet work with any HotRod client.

In Hot Rod "out of the box" absolutely nothing works, as you have to set
key-equivalence="org.infinispan.commons.equivalence.ByteArrayEquivalence"
at the very least just to be able to get the "Get" operation working.

Which is another thing I don't like, but probably unrelated as it
looks like we epxect people to use our "server configurations" which
will have different settings.

>
> Assuming we'll have for HotRod key partitioners soon,
> AffinityPartitioner still won't work with remote caches, where we want
> to keep the key as a byte[]. Instead, we would need a key partitioner
> that knows protobuf and can parse a single field from the object.
>
> So I'd like to withdraw my support for making AffinityPartitioner the default :)
>
> Cheers
> Dan
>
>
> On Fri, Aug 19, 2016 at 5:20 PM, Dan Berindei <dan.berindei at gmail.com> wrote:
>> On Thu, Aug 18, 2016 at 6:12 PM, Sanne Grinovero <sanne at infinispan.org> wrote:
>>> On 18 August 2016 at 14:44, Dan Berindei <dan.berindei at gmail.com> wrote:
>>>> First, a question: doesn't query already configure its internal caches
>>>> to use the AffinityPartitioner? Does it need the application caches to
>>>> have AffinityPartitioner enabled, too?
>>>
>>> No, we may provide reasonable defaults so that demos and tutorials
>>> work out of the box but the general recommendation is that people make
>>> their own configuration, as they will need to tune things, or add
>>> CacheStores, etc..
>>>
>>
>> I see... I wonder if we could at least have some templates for the
>> user to extend, without having to copy a bunch of stuff from
>> tutorials?
>>
>>
>>>> I'm ok with making AffinityPartitioner the default key partitioner:
>>>> the less configuration the user has to change to make things work, the
>>>> better. I'm also ok with changing it to use delegation, to make it
>>>> easy for user to compose it with another key partitioner. We should
>>>> just run some benchmarks first to check it doesn't affect performance
>>>> for repl mode reads.
>>>
>>> Ok!
>>> https://issues.jboss.org/browse/ISPN-6956
>>>
>>> I'll send a PR; regarding benchmarking I'll take the optimistic path:
>>> we'll test it after it's integrated, worst case we can take it out.
>>>
>>>> But I feel users should be able to disable it if they want to, so I
>>>> wouldn't always wrap user partitioners. Also, usually less magic ==
>>>> better. Query can validate the configuration and warn the user if the
>>>> configuration doesn't have AffinityPartitioner in place.
>>>
>>> I disagree on this as this would require more options, which make it
>>> harder to use, and there's no good enough reason to give away the
>>> option to disable this; especially as it kills other features.
>>>
>>
>> Well, grouping isn't enabled by default, even though that means
>> AdvancedCache.getGroup(String) doesn't work with the default
>> configuration. Sure, AffinityPartitioner's overhead is probably lower
>> than GroupingPartitioner's for non-targeted keys, but would you want
>> to wrap all key partitioners with AffinityPartitioner if we were
>> already wrapping them all with GroupingPartitioner?
>>
>> TBH I don't know why a user would want to customize the key
>> partitioner when he could implement AffinityTaggedKey instead...
>>
>>
>>> BTW I never mentioned Query :)
>>
>> You did mention your "evil" plans to improve query performance :D
>>
>>
>>> We happen to use this functionality in Query but as I mentioned other
>>> teams are looking forward to use this functionality, so I think we
>>> should promote this as a public API, a new feature of Infinispan core.
>>>
>>
>> AffinityTaggedKey was already in the public API, even before
>> AffinityPartitioner was the default. So now we have these options for
>> influencing the key distribution:
>> * Implementing AffinityTaggedKey
>> * Implementing a custom KeyPartitioner
>> * Grouping: annotating a key method with @Group, or implementing Grouper
>> * Generating random keys and mapping them to nodes with KeyAffinityService
>>
>>
>>> So since you want to be able to disable it I won't wrap all user
>>> configured implementations in my PR for ISPN-6956, but I'd prefer it
>>> if as a second step you would reconsider and allow me to wrap them
>>> all.
>>>
>>
>> Feel free to open a separate PR for this, I'm definitely not going to
>> block it from going in.
>>
>> Cheers
>> Dan
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev