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

[Dan Berindei]

Is there any way that AffinityPartitioner could work with HotRod, though?


On Fri, Aug 19, 2016 at 5:44 PM, Sanne Grinovero <sanne at infinispan.org> wrote:
> 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
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev