I don't think it would be that much harder to add an option for a user-specified delimiter than it would to hardcode ':' or '|'. I think the utility of that would be much higher, and that it would be a much better candidate for inclusion in geode-core. Otherwise, I think it should live in an "examples" or "utilities" package or repo.
Galen On Wed, Jun 7, 2017 at 12:47 PM, Jacob Barrett <jbarr...@pivotal.io> wrote: > In regard to sending the configuration state to the clients for the > partition resolver maybe the config should be a domain object independent > of the business object rather than the serialized form of the partition > resolver itself. The rational for this would be that non-Java clients could > get the config as PDX (or some other language neutral scheme) and > initialize their native language version of the partition resolver with > this config. > > -Jake > > > Sent from my iPhone > > > On Jun 7, 2017, at 11:42 AM, Darrel Schneider <dschnei...@pivotal.io> > wrote: > > > > 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. > >>>>>>> > >>>>>>> > >>> > >>> > >> >
