Replies inline On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith <dsm...@pivotal.io> wrote:
> Replies inline. > > On Mon, Sep 25, 2017 at 10:54 AM, Brian Baynes <bbay...@pivotal.io> wrote: > > > Thanks for your thoughts, Dan. Some additional info, taking your items # > > by #: > > > > 1) correlationID was put in with the thought that we could support > > out-of-order messages in a future version. You have any input on that > > plan? > > > > This actually brings up another point I was going to ask about. I don't see > any version information in the protocol. How will we handle adding new > features to this protocol? Do the clients and servers negotiate which > version of the protocol to use somehow? > > I think there should be a plan in place for making changes to the protocol > and supporting old clients. Given that, we shouldn't add fields that aren't > actually used into the existing version of the protocol. When we introduce > new features into the protocol, that's the point at which we should add the > fields to support that feature. > [UK] - Protobuf allows for the amending of messages. As soon as the protocol changes significantly the "magic" number will have to be incremented, indicating a new (non-backward compatible) version of the protocol. This of course has bigger problems, where Java does not allow for multiple versions of the same class to be loaded, so a server could run only 1 version of Protobuf messages at a time. > > > 2) Create/destroy region will be added after GA v1.0, so these messages > > should be removed before GA. > > > > Sounds good. > > 3) Idea has been that GetRegion would provide limited metadata on a region, > > similar to what the REST API does. Do see your point on "data policy" > and > > "persistent" being redundant. > > > > I'm not sure I really see the use case for this message or the specific > metadata we are providing, but if someone wants this info then I guess it's > fine. You should probably make sure you are not leaking information that a > user is not supposed to have access to. Like is it ok for GetRegion to > return details of a a region or confirm a region exists if a user does not > have read access to that region? > [UK] I'm off the opinion that the Region*Admin*Service needs to protect itself. IF it receives a request for which the user is not authorized to view, it should either fail that service request or redact the information that is not visible. Making the protocol interface (or any other interface) responsible to check and maintain the relevant authorization levels is not a solution that we should support. With the new protocol interface it was envisioned that any user could extend the functionality using their serialization framework of choice. If you then leave it up to the implementation to enforce the authorization level, we are opening ourselves up to many, many issues and security flaws. > > 4) This could go either way -- we've gone with ease-of-use for clients, > > thinking it's not worth the added complexity for a potentially small > > over-the-wire savings. Would be interested in a good/strong argument for > > the other approach. > > > > Seems reasonable. > [UK] We believe that consistency in return value should override any performance enhancement that we would implement. i.e If an operation, like getAll(), requests 100000 entries, we should return 100000 responses. (bulk of course). The same should apply for putAll(). If I request 100000 entries to be added, one has to (which 100% certainty) know which entries were applied and which were not. For the failures, understanding why they failed is a bigger benefit than receiving a PartialPutAllException. > > > > Finally, we'll be working on a "how to implement a client" document soon, > > including the details you mention. We'd also like to have a simple > client > > implemented in Go to go along with the "how-to". > > > > > > -Brian > > > > On Thu, Sep 21, 2017 at 10:48 AM, Dan Smith <dsm...@pivotal.io> wrote: > > > > > I'm curious about few things I see in the .proto files. > > > > > > 1) I see there is a correlationId in the MessageHeader definition. What > > is > > > that used for? I remember we had a discussion a while back where I > > thought > > > we had decided that might not be not necessary? > > > > > > 2) I also see a CreateRegionRequest and DestroyRegionRequest in the > > .proto > > > files. Are those actually going to be part of the 1.0 GA? Should these > be > > > removed? > > > > > > 3) The GetRegion command seems like it is returning either too much > > > information or to little. It returns some of the attributes of the > > region, > > > like data policy, scope, whether it's persistent (duplicate of data > > > policy?). What is this command for, and should it really be returning > > this > > > information which seems irrelevant to the client? > > > > > > 4) For GetAll, PutAll, the old client server protocol did not return > the > > > keys in the response, it just sent back the results in the same order > as > > > the request. This saves some data on the wire. I"m not sure if it's > worth > > > complexity for this new protocol or not. > > > > > > Looking forward to seeing some more information about how to actually > use > > > these messages to communicate with a server - IE what type of > connection > > > should I create, how SSL works, how authentication works, etc. > > > > > > -Dan > > > > > > On Fri, Sep 15, 2017 at 5:50 PM, Brian Baynes <bbay...@pivotal.io> > > wrote: > > > > > > > You can find them in the code, but we'll be providing better > > > documentation > > > > on the messages shortly. The proto files have the message definitions > > and > > > > they're pretty straightforward, but we'll have a more user-friendly > > > > write-up soon. > > > > > > > > > > > > On Sep 15, 2017 5:27 PM, "Dan Smith" <dsm...@pivotal.io> wrote: > > > > > > > > What's the best place to look for more details on the specific > protocol > > > for > > > > the v1.0 messages? The other pages on https://cwiki.apache.org/ > > > > confluence/display/GEODE/New+Client+Server+Protocol? Or directly in > > the > > > > code somewhere? > > > > > > > > -Dan > > > > > > > > On Fri, Sep 15, 2017 at 11:15 AM, Brian Baynes <bbay...@pivotal.io> > > > wrote: > > > > > > > > > Greetings, friends of Geode. > > > > > > > > > > Work has been progressing on the new client/server protocol for > Geode > > > and > > > > > we're approaching a GA v1.0. Completed work/features include > > put/get, > > > > > putAll, getAll, remove, one-way SSL, authentication and > > authorization, > > > > and > > > > > support for primitive types and JSON documents as values (saved in > > PDX > > > on > > > > > the server). > > > > > > > > > > Invite you to review the road map toward GA v1.0, the features > > proposed > > > > for > > > > > post-v1.0, and give us your feedback! (Directly in Confluence or > > here > > > on > > > > > dev@geode.apache.org) > > > > > > > > > > New Client/Server Protocol - Road Map, Proposed > > > > > <https://cwiki.apache.org/confluence/display/GEODE/Road+ > > > Map%2C+Proposed> > > > > > > > > > > > > > > > Thanks for your input, > > > > > > > > > > Brian > > > > > > > > > > > > > > > -- Kindest Regards ----------------------------- *Udo Kohlmeyer* | *Snr Solutions Architect* |*Pivotal* *Mobile:* +61 409-279-160 | ukohlme...@pivotal.io <http://www.gopivotal.com/> www.pivotal.io
