On Thu, Aug 18, 2016 at 6:12 PM, Sanne Grinovero <sa...@infinispan.org> wrote: > On 18 August 2016 at 14:44, Dan Berindei <dan.berin...@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@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev