You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Brian Baynes <bb...@pivotal.io> on 2017/09/15 18:15:08 UTC
New client/server protocol - seeking feedback
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
Re: New client/server protocol - seeking feedback
Posted by Hitesh Khamesra <hi...@yahoo.com.INVALID>.
+1
On Monday, October 2, 2017, 11:14:55 AM PDT, Jacob Barrett <jb...@pivotal.io> wrote:
A change to a message should just be a new message, no need to version it.
Clients and severs could negotiate the messages they support or attempt the
message they support and fallback to an alternative if the server rejects
it. Consider Put and PutEx (ignore the names):
Put ( Key, Value )
PutEx (Key, Value, SomethingElse )
The client could try PutEx but if rejected by older server and
SomethingElse is not important to its operation it could try Put instead.
Alternatively the server could be queried or a list of supported message
IDs in which it could return only PutEx and the older client could make a
decision easier as to whether or not it can talk to the server.
Although one could argue these are district operation that should be
defined as independent messages anyway. Think clean OO design in your
message design. The message should have significantly change behavior
because of a single parameter otherwise it is really a new thing and should
be defined as a new message.
The short answer is that version numbers make for a nightmare of
compatibility especially when interleaving releases and maintenance
releases. Look at our current protocol and the gaps we leave in the ordinal
numbering to avoid this issue. Let's not make that same mistake.
As for interleaving requests and responses, this should be layered in the
protocol. The top layer should only deal with serial request/response. Let
a lower layer encapsulate the upper level in a multiplexed manor. The naive
layer could just open additional sockets to achieve interleaving, while an
advanced approach would create sub channels on the socket and forward to
the appropriate upper later session. All very easily achieved with Netty.
When the client connects it could negotiate if the server supports the
channel method, just like we negotiate SSL or authentication. The other
benefit to this approach is you don't have an unused field in your protocol
for clients that don't want to implement something so complex.
-Jake
On Mon, Oct 2, 2017 at 10:22 AM Michael Stolz <ms...@pivotal.io> wrote:
> We should check that it is actually safe to add fields.
> If it isn't we're likely to have a lot of versioning to do.
>
> --
> Mike Stolz
> Principal Engineer, GemFire Product Lead
> Mobile: +1-631-835-4771 <(631)%20835-4771>
>
> On Mon, Sep 25, 2017 at 5:25 PM, Galen O'Sullivan <go...@pivotal.io>
> wrote:
>
> > Replies inline.
> >
> > On Mon, Sep 25, 2017 at 12:13 PM, Udo Kohlmeyer <uk...@pivotal.io>
> > wrote:
> >
> > > Replies inline
> > > On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith <ds...@pivotal.io> wrote:
> > >
> > > > 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.
> > >
> >
> > We have to be careful about how we extend protobuf messages, though. I'm
> > not sure exactly what's safe to do, but it should at least be safe to add
> > fields (assuming they don't change existing behavior -- we'll have to
> think
> > about this) and ignore old fields (which is how you would remove a
> > now-unused field). It's fairly simple to add new operations without any
> > interesting breakages - they'll fail with older servers and not be sent
> > with older clients. I think adding new operations is a pretty good way to
> > add features that don't require a real rework of the protocol. For those
> > that do, we can version the initial byte.
> >
>
Re: New client/server protocol - seeking feedback
Posted by Jacob Barrett <jb...@pivotal.io>.
On Mon, Oct 2, 2017 at 1:19 PM Udo Kohlmeyer <uk...@pivotal.io> wrote:
> How to multiplex
> the different major version messages is still up for design and
> implementation though.
>
>
Than I think to Dan's question the correlation ID should go away now until
a design is determined. Adding it because we had it before doesn't make
sense if we are considering different approaches to multiplexing.
-Jake
Re: New client/server protocol - seeking feedback
Posted by Udo Kohlmeyer <uk...@pivotal.io>.
+1 to what Jake said. Our approach is exactly what your preference is. The
adding of messages would new operations and variations on the operation.
PUT will be different to PUT_WITH_CALLBACK. Even in the backend the
processing of the messages will be handled by a different
operationsHandlers.
With the addition of the HANDSHAKE message GEODE-3705
<https://issues.apache.org/jira/browse/GEODE-3705> we hope to be able to
negotiate supported versions and functionality up-front, before processing
any operational messages.
In addition to that, major version changes will only be introduced with
non-backwards compatible changes to the message structure. How to multiplex
the different major version messages is still up for design and
implementation though.
--Udo
On Mon, Oct 2, 2017 at 12:37 PM, Michael William Dodge <md...@pivotal.io>
wrote:
> From my days using Win32 APIs, I think fixing Foo() with FooEx() is an
> anti-pattern. But that's not to say that "version 37 fixes the parameters
> to Foo() and in no other way changes anything" is any better. I see the
> version as useful for determining the structure of the protocol, not the
> specifics of a message per se.
>
> One of the disadvantages of using versions is that it can lead to
> spaghetti code such as cascading if statements to handle different versions
> of any given message. I worry that having the client and server negotiate
> which messages they are going to use would also be a significant addition
> of complexity.
>
> Sarge
>
> > On 2 Oct, 2017, at 11:14, Jacob Barrett <jb...@pivotal.io> wrote:
> >
> > A change to a message should just be a new message, no need to version
> it.
> > Clients and severs could negotiate the messages they support or attempt
> the
> > message they support and fallback to an alternative if the server rejects
> > it. Consider Put and PutEx (ignore the names):
> > Put ( Key, Value )
> > PutEx (Key, Value, SomethingElse )
> > The client could try PutEx but if rejected by older server and
> > SomethingElse is not important to its operation it could try Put instead.
> > Alternatively the server could be queried or a list of supported message
> > IDs in which it could return only PutEx and the older client could make a
> > decision easier as to whether or not it can talk to the server.
> >
> > Although one could argue these are district operation that should be
> > defined as independent messages anyway. Think clean OO design in your
> > message design. The message should have significantly change behavior
> > because of a single parameter otherwise it is really a new thing and
> should
> > be defined as a new message.
> >
> > The short answer is that version numbers make for a nightmare of
> > compatibility especially when interleaving releases and maintenance
> > releases. Look at our current protocol and the gaps we leave in the
> ordinal
> > numbering to avoid this issue. Let's not make that same mistake.
> >
> >
> >
> > As for interleaving requests and responses, this should be layered in the
> > protocol. The top layer should only deal with serial request/response.
> Let
> > a lower layer encapsulate the upper level in a multiplexed manor. The
> naive
> > layer could just open additional sockets to achieve interleaving, while
> an
> > advanced approach would create sub channels on the socket and forward to
> > the appropriate upper later session. All very easily achieved with Netty.
> > When the client connects it could negotiate if the server supports the
> > channel method, just like we negotiate SSL or authentication. The other
> > benefit to this approach is you don't have an unused field in your
> protocol
> > for clients that don't want to implement something so complex.
> >
> >
> > -Jake
> >
> >
> >
> >
> >
> > On Mon, Oct 2, 2017 at 10:22 AM Michael Stolz <ms...@pivotal.io> wrote:
> >
> >> We should check that it is actually safe to add fields.
> >> If it isn't we're likely to have a lot of versioning to do.
> >>
> >> --
> >> Mike Stolz
> >> Principal Engineer, GemFire Product Lead
> >> Mobile: +1-631-835-4771 <(631)%20835-4771>
> >>
> >> On Mon, Sep 25, 2017 at 5:25 PM, Galen O'Sullivan <
> gosullivan@pivotal.io>
> >> wrote:
> >>
> >>> Replies inline.
> >>>
> >>> On Mon, Sep 25, 2017 at 12:13 PM, Udo Kohlmeyer <ukohlmeyer@pivotal.io
> >
> >>> wrote:
> >>>
> >>>> Replies inline
> >>>> On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith <ds...@pivotal.io>
> wrote:
> >>>>
> >>>>> 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.
> >>>>
> >>>
> >>> We have to be careful about how we extend protobuf messages, though.
> I'm
> >>> not sure exactly what's safe to do, but it should at least be safe to
> add
> >>> fields (assuming they don't change existing behavior -- we'll have to
> >> think
> >>> about this) and ignore old fields (which is how you would remove a
> >>> now-unused field). It's fairly simple to add new operations without any
> >>> interesting breakages - they'll fail with older servers and not be sent
> >>> with older clients. I think adding new operations is a pretty good way
> to
> >>> add features that don't require a real rework of the protocol. For
> those
> >>> that do, we can version the initial byte.
> >>>
> >>
>
>
--
Kindest Regards
-----------------------------
*Udo Kohlmeyer* | *Snr Solutions Architect* |*Pivotal*
*Mobile:* +61 409-279-160 | ukohlmeyer@pivotal.io
<http://www.gopivotal.com/>
www.pivotal.io
Re: New client/server protocol - seeking feedback
Posted by Jacob Barrett <jb...@pivotal.io>.
On Mon, Oct 2, 2017 at 12:37 PM Michael William Dodge <md...@pivotal.io>
wrote:
> From my days using Win32 APIs, I think fixing Foo() with FooEx() is an
> anti-pattern. But that's not to say that "version 37 fixes the parameters
> to Foo() and in no other way changes anything" is any better. I see the
> version as useful for determining the structure of the protocol, not the
> specifics of a message per se.
>
I was hoping someone would catch the Win32 reference there and exactly why
I said ignore the names. The point is that two messages that different in
content are in fact two distinct messages with very very very minor
exceptions.
One of the disadvantages of using versions is that it can lead to spaghetti
> code such as cascading if statements to handle different versions of any
> given message. I worry that having the client and server negotiate which
> messages they are going to use would also be a significant addition of
> complexity.
>
Look at our current code... If version 65, do this, if 72, do this, if 80
do that... It is a mess.
-Jake
Re: New client/server protocol - seeking feedback
Posted by Michael William Dodge <md...@pivotal.io>.
From my days using Win32 APIs, I think fixing Foo() with FooEx() is an anti-pattern. But that's not to say that "version 37 fixes the parameters to Foo() and in no other way changes anything" is any better. I see the version as useful for determining the structure of the protocol, not the specifics of a message per se.
One of the disadvantages of using versions is that it can lead to spaghetti code such as cascading if statements to handle different versions of any given message. I worry that having the client and server negotiate which messages they are going to use would also be a significant addition of complexity.
Sarge
> On 2 Oct, 2017, at 11:14, Jacob Barrett <jb...@pivotal.io> wrote:
>
> A change to a message should just be a new message, no need to version it.
> Clients and severs could negotiate the messages they support or attempt the
> message they support and fallback to an alternative if the server rejects
> it. Consider Put and PutEx (ignore the names):
> Put ( Key, Value )
> PutEx (Key, Value, SomethingElse )
> The client could try PutEx but if rejected by older server and
> SomethingElse is not important to its operation it could try Put instead.
> Alternatively the server could be queried or a list of supported message
> IDs in which it could return only PutEx and the older client could make a
> decision easier as to whether or not it can talk to the server.
>
> Although one could argue these are district operation that should be
> defined as independent messages anyway. Think clean OO design in your
> message design. The message should have significantly change behavior
> because of a single parameter otherwise it is really a new thing and should
> be defined as a new message.
>
> The short answer is that version numbers make for a nightmare of
> compatibility especially when interleaving releases and maintenance
> releases. Look at our current protocol and the gaps we leave in the ordinal
> numbering to avoid this issue. Let's not make that same mistake.
>
>
>
> As for interleaving requests and responses, this should be layered in the
> protocol. The top layer should only deal with serial request/response. Let
> a lower layer encapsulate the upper level in a multiplexed manor. The naive
> layer could just open additional sockets to achieve interleaving, while an
> advanced approach would create sub channels on the socket and forward to
> the appropriate upper later session. All very easily achieved with Netty.
> When the client connects it could negotiate if the server supports the
> channel method, just like we negotiate SSL or authentication. The other
> benefit to this approach is you don't have an unused field in your protocol
> for clients that don't want to implement something so complex.
>
>
> -Jake
>
>
>
>
>
> On Mon, Oct 2, 2017 at 10:22 AM Michael Stolz <ms...@pivotal.io> wrote:
>
>> We should check that it is actually safe to add fields.
>> If it isn't we're likely to have a lot of versioning to do.
>>
>> --
>> Mike Stolz
>> Principal Engineer, GemFire Product Lead
>> Mobile: +1-631-835-4771 <(631)%20835-4771>
>>
>> On Mon, Sep 25, 2017 at 5:25 PM, Galen O'Sullivan <go...@pivotal.io>
>> wrote:
>>
>>> Replies inline.
>>>
>>> On Mon, Sep 25, 2017 at 12:13 PM, Udo Kohlmeyer <uk...@pivotal.io>
>>> wrote:
>>>
>>>> Replies inline
>>>> On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith <ds...@pivotal.io> wrote:
>>>>
>>>>> 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.
>>>>
>>>
>>> We have to be careful about how we extend protobuf messages, though. I'm
>>> not sure exactly what's safe to do, but it should at least be safe to add
>>> fields (assuming they don't change existing behavior -- we'll have to
>> think
>>> about this) and ignore old fields (which is how you would remove a
>>> now-unused field). It's fairly simple to add new operations without any
>>> interesting breakages - they'll fail with older servers and not be sent
>>> with older clients. I think adding new operations is a pretty good way to
>>> add features that don't require a real rework of the protocol. For those
>>> that do, we can version the initial byte.
>>>
>>
Re: New client/server protocol - seeking feedback
Posted by Jacob Barrett <jb...@pivotal.io>.
A change to a message should just be a new message, no need to version it.
Clients and severs could negotiate the messages they support or attempt the
message they support and fallback to an alternative if the server rejects
it. Consider Put and PutEx (ignore the names):
Put ( Key, Value )
PutEx (Key, Value, SomethingElse )
The client could try PutEx but if rejected by older server and
SomethingElse is not important to its operation it could try Put instead.
Alternatively the server could be queried or a list of supported message
IDs in which it could return only PutEx and the older client could make a
decision easier as to whether or not it can talk to the server.
Although one could argue these are district operation that should be
defined as independent messages anyway. Think clean OO design in your
message design. The message should have significantly change behavior
because of a single parameter otherwise it is really a new thing and should
be defined as a new message.
The short answer is that version numbers make for a nightmare of
compatibility especially when interleaving releases and maintenance
releases. Look at our current protocol and the gaps we leave in the ordinal
numbering to avoid this issue. Let's not make that same mistake.
As for interleaving requests and responses, this should be layered in the
protocol. The top layer should only deal with serial request/response. Let
a lower layer encapsulate the upper level in a multiplexed manor. The naive
layer could just open additional sockets to achieve interleaving, while an
advanced approach would create sub channels on the socket and forward to
the appropriate upper later session. All very easily achieved with Netty.
When the client connects it could negotiate if the server supports the
channel method, just like we negotiate SSL or authentication. The other
benefit to this approach is you don't have an unused field in your protocol
for clients that don't want to implement something so complex.
-Jake
On Mon, Oct 2, 2017 at 10:22 AM Michael Stolz <ms...@pivotal.io> wrote:
> We should check that it is actually safe to add fields.
> If it isn't we're likely to have a lot of versioning to do.
>
> --
> Mike Stolz
> Principal Engineer, GemFire Product Lead
> Mobile: +1-631-835-4771 <(631)%20835-4771>
>
> On Mon, Sep 25, 2017 at 5:25 PM, Galen O'Sullivan <go...@pivotal.io>
> wrote:
>
> > Replies inline.
> >
> > On Mon, Sep 25, 2017 at 12:13 PM, Udo Kohlmeyer <uk...@pivotal.io>
> > wrote:
> >
> > > Replies inline
> > > On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith <ds...@pivotal.io> wrote:
> > >
> > > > 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.
> > >
> >
> > We have to be careful about how we extend protobuf messages, though. I'm
> > not sure exactly what's safe to do, but it should at least be safe to add
> > fields (assuming they don't change existing behavior -- we'll have to
> think
> > about this) and ignore old fields (which is how you would remove a
> > now-unused field). It's fairly simple to add new operations without any
> > interesting breakages - they'll fail with older servers and not be sent
> > with older clients. I think adding new operations is a pretty good way to
> > add features that don't require a real rework of the protocol. For those
> > that do, we can version the initial byte.
> >
>
Re: New client/server protocol - seeking feedback
Posted by Michael Stolz <ms...@pivotal.io>.
We should check that it is actually safe to add fields.
If it isn't we're likely to have a lot of versioning to do.
--
Mike Stolz
Principal Engineer, GemFire Product Lead
Mobile: +1-631-835-4771
On Mon, Sep 25, 2017 at 5:25 PM, Galen O'Sullivan <go...@pivotal.io>
wrote:
> Replies inline.
>
> On Mon, Sep 25, 2017 at 12:13 PM, Udo Kohlmeyer <uk...@pivotal.io>
> wrote:
>
> > Replies inline
> > On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith <ds...@pivotal.io> wrote:
> >
> > > 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.
> >
>
> We have to be careful about how we extend protobuf messages, though. I'm
> not sure exactly what's safe to do, but it should at least be safe to add
> fields (assuming they don't change existing behavior -- we'll have to think
> about this) and ignore old fields (which is how you would remove a
> now-unused field). It's fairly simple to add new operations without any
> interesting breakages - they'll fail with older servers and not be sent
> with older clients. I think adding new operations is a pretty good way to
> add features that don't require a real rework of the protocol. For those
> that do, we can version the initial byte.
>
Re: New client/server protocol - seeking feedback
Posted by Galen O'Sullivan <go...@pivotal.io>.
Replies inline.
On Mon, Sep 25, 2017 at 12:13 PM, Udo Kohlmeyer <uk...@pivotal.io>
wrote:
> Replies inline
> On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith <ds...@pivotal.io> wrote:
>
> > 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.
>
We have to be careful about how we extend protobuf messages, though. I'm
not sure exactly what's safe to do, but it should at least be safe to add
fields (assuming they don't change existing behavior -- we'll have to think
about this) and ignore old fields (which is how you would remove a
now-unused field). It's fairly simple to add new operations without any
interesting breakages - they'll fail with older servers and not be sent
with older clients. I think adding new operations is a pretty good way to
add features that don't require a real rework of the protocol. For those
that do, we can version the initial byte.
Re: New client/server protocol - seeking feedback
Posted by Udo Kohlmeyer <uk...@pivotal.io>.
Replies inline
On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith <ds...@pivotal.io> wrote:
> Replies inline.
>
> On Mon, Sep 25, 2017 at 10:54 AM, Brian Baynes <bb...@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 <ds...@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 <bb...@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" <ds...@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 <bb...@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 | ukohlmeyer@pivotal.io
<http://www.gopivotal.com/>
www.pivotal.io
Re: New client/server protocol - seeking feedback
Posted by Dan Smith <ds...@pivotal.io>.
Replies inline.
On Mon, Sep 25, 2017 at 10:54 AM, Brian Baynes <bb...@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.
> 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?
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.
> 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 <ds...@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 <bb...@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" <ds...@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 <bb...@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
> > > >
> > >
> >
>
Re: New client/server protocol - seeking feedback
Posted by Brian Baynes <bb...@pivotal.io>.
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?
2) Create/destroy region will be added after GA v1.0, so these messages
should be removed before GA.
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.
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.
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 <ds...@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 <bb...@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" <ds...@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 <bb...@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
> > >
> >
>
Re: New client/server protocol - seeking feedback
Posted by Dan Smith <ds...@pivotal.io>.
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 <bb...@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" <ds...@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 <bb...@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
> >
>
Re: New client/server protocol - seeking feedback
Posted by Brian Baynes <bb...@pivotal.io>.
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" <ds...@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 <bb...@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
>
Re: New client/server protocol - seeking feedback
Posted by Dan Smith <ds...@pivotal.io>.
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 <bb...@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
>