Thanks for all the feedback. Since this is such a simple resolver with a
fixed delimiter the following changes were made to the original proposal:
1. The StringPrefixPartitionResolver will be in the
"org.apache.geode.cache.util" package.
    This was done to keep it from being viewed as part of the core API. It
is just a simple implementation
    of one of the core API interfaces that user can use if they find it
helpful.
2. The fixed delimiter was changed from ":" to "|". The character still has
the visual appearance of
    delimiting two fields with the benefit of being used less often than
":". Since the delimiter can not
    be configured that makes it more likely someone can use this resolver.

I think the ideas about more complex resolvers are interesting and the
existence of this simple resolver does not prevent other resolver
implementations from also being added to the util package. For example one
that is regex based or spring expression based.

One of the things discovered was that if a PartitionResolver has state (for
example a configured delimiter, or regex, or spring expression) then that
java clients will not be able to single hop to the server region with that
resolver. Our servers send the single hop java clients just the class name
of the PartitionResolver. The instance of the actual resolver is not
serialized to the client. The java client single hop code attempts to
create an instance of it by invoking a no-arg constructor. So any state
stored in the server's resolver instance will be lost. If the resolver
class does not have a no-arg constructor then the java client will be
unable to load an instance and just disable single hop. But if it had two
constructors, say one that has no-arg and configures a default delimiter
and another that take a delimiter as an arg, then in that case the single
hop client would lose the delimiter configured on the server and revert to
the default one from the no-arg constructor. It seems like this could cause
all kinds of bad things to happen. For example in the case of a
StringPrefixPartitionResolver that allows the delimiter to be configured
that one used on this client to route keys to the server would have the
wrong delimiter and complain that the key does not contain the delimiter. I
will file a geode ticket about this issue but wanted to have it on this
thread to help explain why this initial resolver is not configurable.


On Tue, Jun 6, 2017 at 1:20 PM, Jacob Barrett <jbarr...@pivotal.io> wrote:

> I have to agree. Something this trivial an limiting is better suited for a
> blog post or examples somewhere and not a part of the core codebase.
>
> -Jake
>
> On Mon, Jun 5, 2017 at 1:27 PM Udo Kohlmeyer <ukohlme...@pivotal.io>
> wrote:
>
> > My concern with this approach is that we provide an implementation that
> > might be a white elephant and only used by a 1% user base. In addition
> > to that, it does really restrict the user on his keys.
> >
> > Whereas something a little more configurable, like a SPEL
> > implementation, would provide a lot more flexibility. Which means that
> > one could easily change the partition resolver expression upon region
> > creation/configuration.
> >
> > --Udo
> >
> >
> > On 6/5/17 08:46, Jens Deppe wrote:
> > > I like the idea of keeping the configuration 'conventional' and thus
> not
> > > requiring any server configuration. As soon as you need to define a
> regex
> > > on the server you might as well be writing PartitionResolver code.
> > >
> > > As an aside I also think that using regexes to parse keys would also
> > > introduce a measurable performance hit.
> > >
> > > --Jens
> > >
> > > On Mon, Jun 5, 2017 at 8:21 AM, Udo Kohlmeyer <ukohlme...@pivotal.io>
> > wrote:
> > >
> > >> Whilst I like the idea to make something (and yes,
> > >> DelimitedStringPartitionResolver is the simplest), I feel that we
> might
> > >> be able to do one better.
> > >>
> > >> */Could/* one possibly have an /*SPEL*/ <
> https://docs.spring.io/spring
> > >>
> > -framework/docs/current/spring-framework-reference/
> html/expressions.html>-based
> > >> PartitionResolver for this? At least this way, we don't have to rely
> on
> > >> delimiters or a strong knowledge of REGEX.
> > >>
> > >> IMO, this would provide the greatest bang for buck implementation.
> > >>
> > >> --Udo
> > >>
> > >>
> > >>
> > >>
> > >> On 6/2/17 19:15, Jacob Barrett wrote:
> > >>
> > >>> If you implement as regular expression the user doesn't have to
> > reformat
> > >>> their key to a specific format (akin to making them implement a
> > class). I
> > >>> would concat the matching groups for generate the routing key.
> > >>>
> > >>> Consider RegEx: .*\bcorrelation=(\d+).*\bmaybe-something-else=(\w)
> > >>> With Keys:
> > >>> A: my,key;with:any-chars;unique=12345;correlation=678/and,maybe
> > >>> -something-else=a
> > >>> B: my,key;unique=876324;correlation=678;and,maybe-
> something-else=a,foo
> > >>> C:
> > somthing;different=988975;correlation=678;then,maybe-something-else=ba
> > >>>
> > >>> Keys A and B would have routing key '678a'. Key C would have routing
> > key
> > >>> '678b'.
> > >>>
> > >>> -Jake
> > >>>
> > >>>
> > >>>
> > >>> Consider
> > >>>
> > >>> On Fri, Jun 2, 2017 at 4:02 PM Darrel Schneider <
> dschnei...@pivotal.io
> > >
> > >>> wrote:
> > >>>
> > >>> Geode partitioned regions usually partition the data based on the
> key's
> > >>>> hashcode.
> > >>>> You can do your own partitioning by implementing the
> PartitionResolver
> > >>>> interface and configuring it on the partitioned region.
> > >>>>
> > >>>> In some use cases needing to deploy your class that implements
> > >>>> PartitionResolver can be problematic so we would like to find a way
> to
> > >>>> offer partitioning based on a portion of the key (instead of the
> > default
> > >>>> which uses the entire key) that does not require you to implement
> your
> > >>>> own
> > >>>> PartitionResolver and does not require you to deploy your own code
> to
> > do
> > >>>> the custom partitioning.
> > >>>>
> > >>>> Another group of users that do not want to implement
> PartitionResolver
> > >>>> are
> > >>>> non-java clients. So the solution is required to be usable by
> non-java
> > >>>> geode clients without needing to reimplement the client to support a
> > new
> > >>>> feature.
> > >>>>
> > >>>> Another constraint on the solution is for it to be both easy to use
> > and
> > >>>> easy to implement.
> > >>>>
> > >>>> The proposed solution is to provide a class named:
> > >>>>       org.apache.geode.cache.StringPrefixPartitionResolver
> > >>>> This class will implement PartitionResolver and have a default
> > >>>> constructor.
> > >>>> To use it you need to configure a partitioned region's
> > PartitionResolver
> > >>>> using the already existing mechanism for this (api, gfsh, or xml).
> > >>>> The StringPrefixPartitionResolver will require all keys on its
> region
> > to
> > >>>> be
> > >>>> of type String.
> > >>>> It also requires that the string key contains at least one ':'
> > character.
> > >>>> The substring of the key that precedes the first ':' is the prefix
> > that
> > >>>> will be returned from "getRoutingObject".
> > >>>>
> > >>>> An example of doing this in gfsh is:
> > >>>>       create region --name=r1 --type=PARTITION
> > >>>> --partition-resolver=org.apache.geode.cache.StringPrefixPart
> > >>>> itionResolver
> > >>>>
> > >>>> Note that attempting to use a key that is not a String or does not
> > >>>> contain
> > >>>> a ':' will throw an exception. This is to help developers realize
> they
> > >>>> made
> > >>>> a mistake.
> > >>>>
> > >>>> Note that the delimiter is always a ':'. It would be easy to made
> the
> > >>>> delimiter configurable when using apis or xml but currently gfsh
> does
> > not
> > >>>> provide a way to pass parameters to the --partition-resolver create
> > >>>> region
> > >>>> option.
> > >>>>
> > >>>> The only public api change this proposal makes is the new
> > >>>> StringPrefixPartitionResolver class.
> > >>>>
> > >>>>
> >
> >
>

Reply via email to