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

[Dan Berindei]

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