On 19 August 2016 at 15:39, Dan Berindei <dan.berin...@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.berin...@gmail.com> wrote: >> 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 _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
