On Thu, Aug 18, 2016 at 6:12 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
On 18 August 2016 at 14:44, Dan Berindei
<dan.berindei(a)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