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. > > >>>> > > >>>> > > > > >