On Fri, Aug 19, 2016 at 3:44 PM, Sanne Grinovero <sa...@infinispan.org> wrote:
> 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. > This is set for you via inheritance at [1], it won't work if using the HotServer class to start the server in unit tests for example. [1] https://github.com/infinispan/infinispan/blob/master/server/integration/infinispan/src/main/resources/infinispan-defaults.xml Gustavo > > 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 >
_______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
