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

[Gustavo Fernandes]

On Fri, Aug 19, 2016 at 3:44 PM, Sanne Grinovero <sanne at infinispan.org>
wrote:

> On 19 August 2016 at 15:39, Dan Berindei <dan.berindei at 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.berindei at gmail.com>
> wrote:
> >> 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
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/infinispan-dev/attachments/20160819/d619565a/attachment-0001.html