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

[Gustavo Fernandes]

On Fri, Aug 19, 2016 at 3:39 PM, 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.
>
> 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 :)
>
>

+1 for me, I don't think it should be default. Currently the Hot Rod
protocol
is hardcoded to use a hash function to map keys to segments (byte field
in the header to indicate hash function version to use), so C#, C++, JS
clients would not be able to take advantage of the AffinityPartitioner.

Another thing to consider is for query usage, mapping 1 segment to 1
index shard uses lots of resources, and I've been exploring ways of
making this configurable [1], which potentially could require a different
partitioner.

That does not mean it should never be default, we can revisit this later.


Gustavo


[1] https://issues.jboss.org/browse/ISPN-6225



> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/infinispan-dev/attachments/20160822/db44b197/attachment.html