Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

>>>The message header currently is specified to have things like correlation
id, isPartial message, and also metdatadata about whether the key or the
value is JSON.
IsPartialMessage: This flag gives us ability to send partial message without 
serializing the whole key-value(request). lets say I execute function at server 
and function just returns "arraylist of object". And the serialized size of 
""arraylist of object"" is quite big( > 2gb).
metadata: Most of the metadata will be optional. JSON: this is a feature we 
want to introduce, where client can send JSON string and we want to save that 
JSON string into pdx.


  From: Dan Smith 
 To: Udo Kohlmeyer  
Cc: dev@geode.apache.org
 Sent: Monday, May 1, 2017 5:53 PM
 Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
   
I think the current proposal seems to be gloming some things together that
probably belong in different layers.

The message header currently is specified to have things like correlation
id, isPartial message, and also metdatadata about whether the key or the
value is JSON. Fragmenting messages (isPartialMessage) seems like it
belongs at a lower level, and depending on what transport your are using is
probably already handled (TCP, HTTP). Potentially similar issue for
request/response (correlationId)  - if you were going over http that would
already be handled. Whether a value is JSON seems like it belongs at a
higher level that specifies the value serialization format. Not to mention
not all messages have keys and values, and some have more than one.

If wonder if it would make more sense to organize the message structure
into layers that each have their own responsibility - a fragmentation layer
(maybe not necessary), a request/response layer (maybe not necessary, not
needed for all message types), function execution layer, individual
operations layer (put, get), value serialization layer.

Without having nailed down what underlying serialization framework you are
using, talking about field byte sizes, etc. seems a bit premature.

-Dan

On Fri, Apr 28, 2017 at 2:49 PM, Udo Kohlmeyer 
wrote:

> Hi there Geode community,
>
> The new Client-Server protocol proposal is available for review.
>
> It can be viewed and commented on https://cwiki.apache.org/confl
> uence/display/GEODE/New+Client+Server+Protocol
>
> --Udo
>
>


   

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

Absolutely its a implementation detail.
JSON: Surely we can consider ValueHeader. But then every client(and message) 
needs to send that. Using metadata its a optional.


  From: Dan Smith 
 To: Udo Kohlmeyer  
Cc: dev@geode.apache.org
 Sent: Tuesday, May 2, 2017 11:39 AM
 Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
   
> IsPartialMessage: This flag gives us ability to send partial message
> without serializing the whole key-value(request). lets say I execute
> function at server and function just returns "arraylist of object". And the
> serialized size of ""arraylist of object"" is quite big( > 2gb).
>

My point about these fields is that it really seems like this stuff should
be handled by different layers. Ideally you would have a fragmentation
layer that is invisible to people writing specific messages, so that
messages are automatically fragmented if they get to large. Think about how
a TCP socket works - you just write data and it is automatically
fragmented. Or are you expecting each individual message type to have it's
own way to doing fragmentation, but it should set this header down in your
protocol layer? That seems really messy.

JSON: this is a feature we want to introduce, where client can send JSON
> string and we want to save that JSON string into pdx.


Same thing here, JSON support sounds great, but having a header field of
JSON_KEY seems like a hacky way to do that. It seems like that might belong
in your ValueHeader.




On Tue, May 2, 2017 at 10:20 AM, Udo Kohlmeyer 
wrote:

> Hey Dan,
>
> Imo, having a standardized, versioned definition for GET, PUT, PUTALL,
> etc. message, that is encoded/decoded in a manner that multiple clients
> (written in many other languages) can encode/decode these messages, is
> paramount.
>
> Having the standardized operational messages(GET,PUT,etc.) transported
> using the function service vs a more direct operation handler, that is
> another discussion and is something that should be investigated.
>
> My immediate concerns regarding "normal" operations over the function
> service are:
>
>    1. I don't believe the current function service is "stream" enabled,
>    and would require some potential rework for subscription-based operations
>    2. Can the function service handle the extra load?
>    3. Is the function service "lean" enough to sustain acceptable
>    throughput? The current client/server protocol averages around
>    40,000-50,000 messages/second.
>    4. There are some messages that are passed between the client <->
>    locator. Given that the function service is "server" specific, this
>    approach would not work for locators, where a different transport mechanism
>    is required. (but this is not a show stopper if function service proves to
>    be viable)
>    5. How much effort would be required to make the "old" function
>    service, handle the new messages, ensuring that the current behavior is
>    preserved.
>
> As per a previous discussion we had, I believe that the "function-like"
> behavior (retry, HA, write vs read optimized) can incorporated into the
> processing layer on the server. In that way all messages can benefit from
> that behavior. In addition to this, if we have a single mechanism that will
> handle messages, retry, HA, read/write optimizations, is preferable to
> having a few "bespoke" implementations. So either approach (new message
> handling) vs function service, will be preferable.
>
> "*The advantage of this approach is that if someone just builds a driver
> that only supports function execution and whatever serialization framework
> is required to serialize function arguments, they already have an API that
> application developers could use to do pretty much anything they wanted to
> do on the server. Having a Region object with methods like get and put on
> it could just be a little syntatic sugar on top of that.*"
>
> It can be argued that having a standard client/server message, with
> standardized encoding/decoding, is the same as using function execution.
> Both require a little syntactic sugar to add new functionality to an
> already standardized message.
>
> --Udo
> On 5/1/17 17:27, Dan Smith wrote:
>
> I think any new client driver or server we develop might want to
> incorporate function execution at lower level than region operations like
> get and put, etc. We could then easily build operations like GET, PUT,
> PUTALL, etc. on top of that by making them functions. The original client
> protocol isn't designed like that because it pre-dates function execution.
>
> The current function execution API is a little clunky and needs some work.
> But what it does do is provide the fundamental logic to target operations
> at members that host certain keys and retry in the case of failure.
>
> The advantage of this approach is that if someone just builds a driver
> that only supports function execution and whatever serialization framework
> is required to serialize function arguments,

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

We have version at api(put, get etc) level 
https://cwiki.apache.org/confluence/display/GEODE/Message+Structure+and+Definition#MessageStructureandDefinition-RequestHeader.
The client will connect to gemfire server by sending the "byte". That can be 
treated for message serialization.

  From: Michael Stolz 
 To: dev@geode.apache.org 
Cc: Udo Kohlmeyer 
 Sent: Wednesday, May 3, 2017 8:55 AM
 Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
   
I'm not seeing any mention of versioning of the serialization protocol.
Versioning is critical to be able to support change over time. We must
version each layer of serialization. The transport message needs versions,
the payload serialization needs versions.

--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: +1-631-835-4771

On Tue, May 2, 2017 at 8:11 PM, Jacob Barrett  wrote:

> I agree completely with Dan. There is no reason to have flags for value
> encoding type in the message. I would argue that should be part of the
> value serialization layer. If something was placed in the message layer it
> should be more generic and allow for an unrestricted set of encodings by
> some ID.
>
> Object {
> variant ID codec;
> byte[] payload;
> }
>
>
> -Jake
>
>
> Sent from my iPhone
>
> > On May 2, 2017, at 1:42 PM, Dan Smith  wrote:
> >
> > I guess the value of building other messages on top of the function
> service
> > mostly comes into play when we start talking about smarter clients that
> can
> > do single hop. At that point it's really nice to have have a layer that
> > lets us send a message to a single primary, or all of the members that
> host
> > a region etc. It is also nice that right now if I add new function that
> > functionality becomes available to gfsh, REST, Java, and C++ developers
> > automatically.
> >
> > I do agree that the new protocol could build in these concepts, and
> doesn't
> > necessarily have to use function execution to achieve the same results.
> But
> > do at least consider whether new developers will want to add new
> > functionality to the server via functions or via your this new protocol.
> If
> > it's harder to use the new protocol than to write a new function and
> invoke
> > it from the client, then I think we've done something wrong.
> >
> >
> > A couple of other comments, now that I've looked a little more:
> >
> > 1) The list of error codes
> >  ErrorCodeDefinitions>
> > seems really incomplete. It looks like we've picked a few of the possible
> > exceptions geode could throw and assigned them integer ids? What is the
> > rational for the exceptions that are included here vs. other exceptions?
> > Also, not all messages would need to return these error codes.
> >
> > 2) The existing protocol has some functionality even for basic puts that
> is
> > not represented here. Client generate an event id that is associated with
> > the put and send that to the server. These event ids are used to
> guarantee
> > that if a client does put (A, 0) followed by put (A, 1), the resulting
> > value will always be 1, even if the client timed out and retried put (A,
> > 0). The event id prevents the lingered put that timed out on the server
> > from affecting the state. I'm not saying the new protocol has to support
> > this sort of behavior, but you might want to consider whether the current
> > protocol should specify anything about how events are retried.
> >
> > -Dan
>


   

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

Here are the few things we need to consider..
1. key, value, callbackarg can be required to interpret as JSON-to-pdx2. client 
calls "get/getall" api and want return value as JSON. Value was serialized as 
pdx.3. This behavior should be optional, if possible no overhead for others.4. 
"putAll api" can have mixed type values(JSON and numbers). Dan raised about 
this. And may be worth to consider it.
Thus my initial thought was client should indicate this feature at message 
level(metadata), saying, convert pdx value to json or vice-versa.
Any thoughts?
Thanks.HItesh


  From: Jacob Barrett 
 To: dev@geode.apache.org 
Cc: Udo Kohlmeyer 
 Sent: Tuesday, May 2, 2017 8:11 PM
 Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
   
I agree completely with Dan. There is no reason to have flags for value 
encoding type in the message. I would argue that should be part of the value 
serialization layer. If something was placed in the message layer it should be 
more generic and allow for an unrestricted set of encodings by some ID.

Object {
variant ID codec;
byte[] payload;
}


-Jake


Sent from my iPhone

> On May 2, 2017, at 1:42 PM, Dan Smith  wrote:
> 
> I guess the value of building other messages on top of the function service
> mostly comes into play when we start talking about smarter clients that can
> do single hop. At that point it's really nice to have have a layer that
> lets us send a message to a single primary, or all of the members that host
> a region etc. It is also nice that right now if I add new function that
> functionality becomes available to gfsh, REST, Java, and C++ developers
> automatically.
> 
> I do agree that the new protocol could build in these concepts, and doesn't
> necessarily have to use function execution to achieve the same results. But
> do at least consider whether new developers will want to add new
> functionality to the server via functions or via your this new protocol. If
> it's harder to use the new protocol than to write a new function and invoke
> it from the client, then I think we've done something wrong.
> 
> 
> A couple of other comments, now that I've looked a little more:
> 
> 1) The list of error codes
> 
> seems really incomplete. It looks like we've picked a few of the possible
> exceptions geode could throw and assigned them integer ids? What is the
> rational for the exceptions that are included here vs. other exceptions?
> Also, not all messages would need to return these error codes.
> 
> 2) The existing protocol has some functionality even for basic puts that is
> not represented here. Client generate an event id that is associated with
> the put and send that to the server. These event ids are used to guarantee
> that if a client does put (A, 0) followed by put (A, 1), the resulting
> value will always be 1, even if the client timed out and retried put (A,
> 0). The event id prevents the lingered put that timed out on the server
> from affecting the state. I'm not saying the new protocol has to support
> this sort of behavior, but you might want to consider whether the current
> protocol should specify anything about how events are retried.
> 
> -Dan

   

Re: New Client-Server Protocol Proposal

[Hitesh Khamesra]

(Just format change)
Here are the few things we need to consider..
1. key, value, callbackarg can be required to interpret as JSON-to-pdx2. client 
calls "get/getall" api and want return value as JSON. Value was serialized as 
pdx.3. This behavior should be optional, if possible no overhead for others.4. 
"putAll api" can have mixed value types(JSON and numbers etc). Dan raised about 
this. And may be worth to consider it.
Thus my initial thought was client should indicate this feature at message 
level(metadata), saying, convert pdx value to json or vice-versa.

Any thoughts?

Thanks.Hitesh.
  From: Hitesh Khamesra 
 To: "dev@geode.apache.org"  
 Sent: Wednesday, May 3, 2017 10:01 AM
 Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
   
Here are the few things we need to consider..
1. key, value, callbackarg can be required to interpret as JSON-to-pdx2. client 
calls "get/getall" api and want return value as JSON. Value was serialized as 
pdx.3. This behavior should be optional, if possible no overhead for others.4. 
"putAll api" can have mixed type values(JSON and numbers). Dan raised about 
this. And may be worth to consider it.
Thus my initial thought was client should indicate this feature at message 
level(metadata), saying, convert pdx value to json or vice-versa.
Any thoughts?
Thanks.HItesh


      From: Jacob Barrett 
 To: dev@geode.apache.org 
Cc: Udo Kohlmeyer 
 Sent: Tuesday, May 2, 2017 8:11 PM
 Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
  
I agree completely with Dan. There is no reason to have flags for value 
encoding type in the message. I would argue that should be part of the value 
serialization layer. If something was placed in the message layer it should be 
more generic and allow for an unrestricted set of encodings by some ID.

Object {
variant ID codec;
byte[] payload;
}


-Jake


Sent from my iPhone

> On May 2, 2017, at 1:42 PM, Dan Smith  wrote:
> 
> I guess the value of building other messages on top of the function service
> mostly comes into play when we start talking about smarter clients that can
> do single hop. At that point it's really nice to have have a layer that
> lets us send a message to a single primary, or all of the members that host
> a region etc. It is also nice that right now if I add new function that
> functionality becomes available to gfsh, REST, Java, and C++ developers
> automatically.
> 
> I do agree that the new protocol could build in these concepts, and doesn't
> necessarily have to use function execution to achieve the same results. But
> do at least consider whether new developers will want to add new
> functionality to the server via functions or via your this new protocol. If
> it's harder to use the new protocol than to write a new function and invoke
> it from the client, then I think we've done something wrong.
> 
> 
> A couple of other comments, now that I've looked a little more:
> 
> 1) The list of error codes
> <https://cwiki.apache.org/confluence/display/GEODE/RegionAPI#RegionAPI-ErrorCodeDefinitions>
> seems really incomplete. It looks like we've picked a few of the possible
> exceptions geode could throw and assigned them integer ids? What is the
> rational for the exceptions that are included here vs. other exceptions?
> Also, not all messages would need to return these error codes.
> 
> 2) The existing protocol has some functionality even for basic puts that is
> not represented here. Client generate an event id that is associated with
> the put and send that to the server. These event ids are used to guarantee
> that if a client does put (A, 0) followed by put (A, 1), the resulting
> value will always be 1, even if the client timed out and retried put (A,
> 0). The event id prevents the lingered put that timed out on the server
> from affecting the state. I'm not saying the new protocol has to support
> this sort of behavior, but you might want to consider whether the current
> protocol should specify anything about how events are retried.
> 
> -Dan

  

   

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

(Sorry: one more attempt to format this message)

Here are the few things we need to consider..


1. key, value, callbackarg can be required to interpret as JSON-to-pdx.
2. client calls "get/getall" api and want return value as JSON. Value was 
serialized as pdx.
3. This behavior should be optional, if possible no overhead for others.
4. "putAll api" can have mixed type values(JSON and numbers). Dan raised about 
this. And may be worth to consider it.

Thus my initial thought was client should indicate this feature at message 
level(metadata), saying, convert pdx value to json or vice-versa.


Any thoughts?
Thanks.
HItesh



________
From: Hitesh Khamesra 
To: "dev@geode.apache.org"  
Sent: Wednesday, May 3, 2017 10:01 AM
Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal



Here are the few things we need to consider..
1. key, value, callbackarg can be required to interpret as JSON-to-pdx2. client 
calls "get/getall" api and want return value as JSON. Value was serialized as 
pdx.3. This behavior should be optional, if possible no overhead for others.4. 
"putAll api" can have mixed type values(JSON and numbers). Dan raised about 
this. And may be worth to consider it.
Thus my initial thought was client should indicate this feature at message 
level(metadata), saying, convert pdx value to json or vice-versa.
Any thoughts?
Thanks.HItesh


  From: Jacob Barrett 

To: dev@geode.apache.org 
Cc: Udo Kohlmeyer 
Sent: Tuesday, May 2, 2017 8:11 PM
Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
  
I agree completely with Dan. There is no reason to have flags for value 
encoding type in the message. I would argue that should be part of the value 
serialization layer. If something was placed in the message layer it should be 
more generic and allow for an unrestricted set of encodings by some ID.

Object {
variant ID codec;
byte[] payload;
}


-Jake


Sent from my iPhone

> On May 2, 2017, at 1:42 PM, Dan Smith  wrote:
> 
> I guess the value of building other messages on top of the function service
> mostly comes into play when we start talking about smarter clients that can
> do single hop. At that point it's really nice to have have a layer that
> lets us send a message to a single primary, or all of the members that host
> a region etc. It is also nice that right now if I add new function that
> functionality becomes available to gfsh, REST, Java, and C++ developers
> automatically.
> 
> I do agree that the new protocol could build in these concepts, and doesn't
> necessarily have to use function execution to achieve the same results. But
> do at least consider whether new developers will want to add new
> functionality to the server via functions or via your this new protocol. If
> it's harder to use the new protocol than to write a new function and invoke
> it from the client, then I think we've done something wrong.
> 
> 
> A couple of other comments, now that I've looked a little more:
> 
> 1) The list of error codes
> <https://cwiki.apache.org/confluence/display/GEODE/RegionAPI#RegionAPI-ErrorCodeDefinitions>
> seems really incomplete. It looks like we've picked a few of the possible
> exceptions geode could throw and assigned them integer ids? What is the
> rational for the exceptions that are included here vs. other exceptions?
> Also, not all messages would need to return these error codes.
> 
> 2) The existing protocol has some functionality even for basic puts that is
> not represented here. Client generate an event id that is associated with
> the put and send that to the server. These event ids are used to guarantee
> that if a client does put (A, 0) followed by put (A, 1), the resulting
> value will always be 1, even if the client timed out and retried put (A,
> 0). The event id prevents the lingered put that timed out on the server
> from affecting the state. I'm not saying the new protocol has to support
> this sort of behavior, but you might want to consider whether the current
> protocol should specify anything about how events are retried.
> 
> -Dan

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

Good point Dan !! that needs to document.


  From: Dan Smith 
 To: dev@geode.apache.org 
 Sent: Wednesday, May 3, 2017 5:31 PM
 Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
   
Okay  but how do I has an implementer of a driver know what messages
need an event id and which don't? It seems like maybe this belongs with
those message types, rather than in a generic header. Or maybe you need to
start organizing messages into classes - eg messages that change state and
messages that don't and abstracting out commonality.

It's also not clear exactly what the event id should be set to. When do a
change the sequence id? Does it have to be monotonically increasing? What
should the uniqueId be?

-Da

On Wed, May 3, 2017 at 5:07 PM, Udo Kohlmeyer  wrote:

> Correct,
>
> I did miss that. @Dan, if you look at https://cwiki.apache.org/confl
> uence/display/GEODE/Message+Structure+and+Definition#Messa
> geStructureandDefinition-MetaDataforRequests specifies how we provide
> EventId information.
>
>
>
> On 5/3/17 09:53, Bruce Schuchardt wrote:
>
>> I believe Hitesh put EventId in the metadata section.
>>
>> Le 5/2/2017 à 2:22 PM, Udo Kohlmeyer a écrit :
>>
>>> We are considering the function service, but again, this should not
>>> detract from the proposed message specification proposal.
>>>
>>> You are also correct in your observation of list of error codes not
>>> being complete nor exhaustive. Maybe the first page needs to highlight that
>>> this is a proposal and does not contain all the error codes that we could
>>> per api.
>>>
>>> As for the EventId, we will look into this and update the document
>>> accordingly.
>>>
>>> --Udo
>>>
>>>
>>> On 5/2/17 13:42, Dan Smith wrote:
>>>
 I guess the value of building other messages on top of the function
 service mostly comes into play when we start talking about smarter clients
 that can do single hop. At that point it's really nice to have have a layer
 that lets us send a message to a single primary, or all of the members that
 host a region etc. It is also nice that right now if I add new function
 that functionality becomes available to gfsh, REST, Java, and C++
 developers automatically.

 I do agree that the new protocol could build in these concepts, and
 doesn't necessarily have to use function execution to achieve the same
 results. But do at least consider whether new developers will want to add
 new functionality to the server via functions or via your this new
 protocol. If it's harder to use the new protocol than to write a new
 function and invoke it from the client, then I think we've done something
 wrong.


 A couple of other comments, now that I've looked a little more:

 1) The list of error codes  seems
 really incomplete. It looks like we've picked a few of the possible
 exceptions geode could throw and assigned them integer ids? What is the
 rational for the exceptions that are included here vs. other exceptions?
 Also, not all messages would need to return these error codes.

 2) The existing protocol has some functionality even for basic puts
 that is not represented here. Client generate an event id that is
 associated with the put and send that to the server. These event ids are
 used to guarantee that if a client does put (A, 0) followed by put (A, 1),
 the resulting value will always be 1, even if the client timed out and
 retried put (A, 0). The event id prevents the lingered put that timed out
 on the server from affecting the state. I'm not saying the new protocol has
 to support this sort of behavior, but you might want to consider whether
 the current protocol should specify anything about how events are retried.

 -Dan

>>>
>>>
>>>
>>
>

   

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

Hi Anthony:

Help me to understand data chunking here?

>> bytes => arbitrary byte[] that can be chunked

Message => MessageHeader MessageBody

So lets say we want to send long byte[] into two chunks, then we will send two 
messages? And other side will combine those two messages using "correlationId" ?

Thanks.
HItesh





From: Anthony Baker 
To: dev@geode.apache.org 
Cc: Hitesh Khamesra 
Sent: Wednesday, May 3, 2017 5:42 PM
Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal




> On May 3, 2017, at 1:33 PM, Galen M O'Sullivan  wrote:
> 
> On Tue, May 2, 2017 at 11:52 AM, Hitesh Khamesra <
> hitesh...@yahoo.com.invalid> wrote:
> 
>> Absolutely its a implementation detail.
>> 
> This doesn't answer Dan's comment. Do you think fragmentation should be
> taken care of by the TCP layer or the protocol should deal with it
> specifically?

There’s some really good feedback and discussion in this thread!  Here are a 
few thoughts:

1) Optional metadata should be used for fields that are generally applicable 
across all messages.  If a metadata field is required or only applies to a 
small set of messages, it should become part of a message definition.  Of 
course there’s some grey area here.

2) I think we should pull out the message fragmentation support to avoid some 
significant complexity.  We can later add a fragmentation / envelope layer on 
top without disrupting the current proposal.  I do think we should add the 
capability for chunking data (more on that below).

3) I did not find any discussion of message pipelining (queuing multiple 
requests on a socket without waiting for a response) or out-of-order responses. 
 What is the plan for these capabilities and how will that affect consistency?  
What about retries?

4) Following is an alternative definition with these characteristics:

- Serialized data can either be primitive or encoded values.  Encoded values 
are chunked as needed to break up large objects into a series of smaller parts.
- Because values can be chunked, the size field is removed.  This allows the 
message to be streamed to the socket incrementally.
- The apiVersion is removed because we can just define a new body type with a 
new apiId (e.g. GetRequest2 with aipId = 1292).
- The GetRequest tells the server what kind of encoding the client is able to 
understand.
- The metadata map is not used for fields that belong in the message body.  I 
think it’s much easier to write a spec without if statements :-)

Message => MessageHeader MessageBody

MessageHeader => correlationId metadata
correlationId => integer
metadata => count (key value)*
count => integer
key => string
value => string

MessageBody => apiId body
apiId => integer
body => (see specific definitions)

GetRequest => 0 acceptEncoding key
0 => the API id
acceptEncoding => (define some encodings for byte[], JSON, PDX, *, etc)
key => EncodedValue

GetResponse => 1 value
1 => the API id
value => EncodedValue

PutRequest => 2 eventId key value
2 => the API id
eventId => clientId threadId sequenceId
clientId => string
threadId => integer
sequenceId => integer
key => EncodedValue
value => EncodedValue

EncodedValue => encoding (boolean | integer | number | string | ((length 
bytes)* 0))
encoding => (define some encodings for byte[], JSON, PDX, *, etc)
boolean => TRUE or FALSE
integer => a signed integer value
number => a decimal value corresponding to IEEE 754
string => UTF-8 text
bytes => arbitrary byte[] that can be chunked


Anthony

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

And len 0 would indicate end of the message? 


a. Now these two chunks will go continuous. 


b. If its PDX encoded then pdx header(1byte:pdxid 4byte:len 4byte:typeId) 
requires size of all pdx serialized bytes. So we know "size" of data upfront 
here. 


c. Lets say region value is just long byte[]:  Then we have "size" to send the 
message.


So in both cases we know the "size" of serialized bytes(payload). So possibly 
we don't need to chunk that message and let tcp take care of it?


It seems we should walk through some more usecases to understand this better.



Thanks.
Hitesh



________
From: Anthony Baker 
To: Hitesh Khamesra  
Cc: "dev@geode.apache.org" 
Sent: Thursday, May 4, 2017 11:20 AM
Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal



There would be one Message containing a single MessageHeader and a single 
MessageBody.  A PDX EncodedValue containing 1242 bytes that are chunked would 
look something like this:

PDX 1000 byte[1000] 242 byte[242] 0


Anthony



> On May 4, 2017, at 10:38 AM, Hitesh Khamesra  wrote:
> 
> Hi Anthony:
> 
> Help me to understand data chunking here?
> 
>>> bytes => arbitrary byte[] that can be chunked
> 
> Message => MessageHeader MessageBody
> 
> So lets say we want to send long byte[] into two chunks, then we will send 
> two messages? And other side will combine those two messages using 
> "correlationId" ?
> 
> Thanks.
> HItesh
> 
> 
> 
> 
> 
> From: Anthony Baker 
> To: dev@geode.apache.org 
> Cc: Hitesh Khamesra 
> Sent: Wednesday, May 3, 2017 5:42 PM
> Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
> 
> 
> 
> 
>> On May 3, 2017, at 1:33 PM, Galen M O'Sullivan  wrote:
>> 
>> On Tue, May 2, 2017 at 11:52 AM, Hitesh Khamesra <
>> hitesh...@yahoo.com.invalid> wrote:
>> 
>>> Absolutely its a implementation detail.
>>> 
>> This doesn't answer Dan's comment. Do you think fragmentation should be
>> taken care of by the TCP layer or the protocol should deal with it
>> specifically?
> 
> There’s some really good feedback and discussion in this thread!  Here are a 
> few thoughts:
> 
> 1) Optional metadata should be used for fields that are generally applicable 
> across all messages.  If a metadata field is required or only applies to a 
> small set of messages, it should become part of a message definition.  Of 
> course there’s some grey area here.
> 
> 2) I think we should pull out the message fragmentation support to avoid some 
> significant complexity.  We can later add a fragmentation / envelope layer on 
> top without disrupting the current proposal.  I do think we should add the 
> capability for chunking data (more on that below).
> 
> 3) I did not find any discussion of message pipelining (queuing multiple 
> requests on a socket without waiting for a response) or out-of-order 
> responses.  What is the plan for these capabilities and how will that affect 
> consistency?  What about retries?
> 
> 4) Following is an alternative definition with these characteristics:
> 
> - Serialized data can either be primitive or encoded values.  Encoded values 
> are chunked as needed to break up large objects into a series of smaller 
> parts.
> - Because values can be chunked, the size field is removed.  This allows the 
> message to be streamed to the socket incrementally.
> - The apiVersion is removed because we can just define a new body type with a 
> new apiId (e.g. GetRequest2 with aipId = 1292).
> - The GetRequest tells the server what kind of encoding the client is able to 
> understand.
> - The metadata map is not used for fields that belong in the message body.  I 
> think it’s much easier to write a spec without if statements :-)
> 
> Message => MessageHeader MessageBody
> 
> MessageHeader => correlationId metadata
>correlationId => integer
>metadata => count (key value)*
>count => integer
>key => string
>value => string
> 
> MessageBody => apiId body
>apiId => integer
>body => (see specific definitions)
> 
> GetRequest => 0 acceptEncoding key
>0 => the API id
>acceptEncoding => (define some encodings for byte[], JSON, PDX, *, etc)
>key => EncodedValue
> 
> GetResponse => 1 value
>1 => the API id
>value => EncodedValue
> 
> PutRequest => 2 eventId key value
>2 => the API id
>eventId => clientId threadId sequenceId
>clientId => string
>threadId => integer
>sequenceId => integer
>key => EncodedValue
>value => EncodedValue
> 
> EncodedValue => encoding (boolean | integer | number | string | ((length 
> bytes)* 0))
>encoding => (define some encodings for byte[], JSON, PDX, *, etc)
>boolean => TRUE or FALSE
>integer => a signed integer value
>number => a decimal value corresponding to IEEE 754
>string => UTF-8 text
>bytes => arbitrary byte[] that can be chunked
> 
> 
> Anthony

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

>>> a. Now these two chunks will go continuous. 

>>They would appear continuous to the object serialization layer.

One benefit of messageHeader with chunk is that, it gives us ability to write 
different messages(multiplexing) on same socket. And if thread is ready it can 
write its message. Though we are not there yet, but that will require for 
single socket async architecture.





From: Jacob Barrett 
To: dev@geode.apache.org 
Cc: Anthony Baker 
Sent: Thursday, May 4, 2017 12:48 PM
Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal





Sent from my iPhone

> On May 4, 2017, at 12:03 PM, Hitesh Khamesra  
> wrote:
> 
> And len 0 would indicate end of the message? 
> 
> 
> a. Now these two chunks will go continuous. 

They would appear continuous to the object serialization layer.



> 
> 
> b. If its PDX encoded then pdx header(1byte:pdxid 4byte:len 4byte:typeId) 
> requires size of all pdx serialized bytes. So we know "size" of data upfront 
> here. 

We could define the InputSource for the Value part such that 
InputSource.getLength() could return a known length or -1 if length is unknown. 
If length is reasonable then the object could be encoded with a single chuck of 
size InputSource.getLength() followed by a 0 chunk. 

Clients are likely dealing with domain objects where the serialize length is 
not known until serialization is complete. This would require buffering to get 
the length. Buffering adds heap pressure and latency.

-Jake

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

Basically, thread/layer should not hold any resources while serializing the 
object or chunk.  We should be able to see this flow (ms1-chunk1, msg2-chunk1, 
ms1-chunk2, msg3-chunk, msg2-chunk2, so on ...)



On other pdx note: to de-serialize the pdx we need length of serialized bytes, 
so that we can read field offset from serialized stream, and then can read 
field value. Though, I can imagine with the help of pdxType, we can interpret 
serialized stream.






From: Jacob Barrett 
To: dev@geode.apache.org; Hitesh Khamesra  
Cc: Anthony Baker 
Sent: Thursday, May 4, 2017 2:14 PM
Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal



> One benefit of messageHeader with chunk is that, it gives us ability to
> write different messages(multiplexing) on same socket. And if thread is
> ready it can write its message. Though we are not there yet, but that will
> require for single socket async architecture.
>

I wouldn't tackle that at this layer. I would suggest adding a layer
between the message and TCP that creates channels that are opaque to the
message layer above. The message layer wouldn't know if it was talking to
multiple sockets to the client or single socket with multiple channels.


-Jake

Re: [gemfire-dev] New Client-Server Protocol Proposal

[Hitesh Khamesra]

0. In first phase we are not doing chunking/fragmentation. And even this will 
be option for 
client.(https://cwiki.apache.org/confluence/display/GEODE/Message+Structure+and+Definition#MessageStructureandDefinition-Protocolnegotiation)
1. Are you refereeing websocket/spdy? But I think we are talking almost same 
thing, may be push isPartialMessage flag with chunk length(Anthony's example 
below) ?
2. That's the part of the problem. Even if you need to serialize the "String", 
you need to write length first and then need to write serialized utf bytes. We 
can implement chunked input stream and can de-serialize the object as it is 
coming (DataSerializable.fromData(ChunkedStream)). 




  From: Jacob Barrett 
 To: dev@geode.apache.org; Hitesh Khamesra  
Cc: Anthony Baker 
 Sent: Friday, May 5, 2017 7:29 AM
 Subject: Re: [gemfire-dev] New Client-Server Protocol Proposal
   

On Thu, May 4, 2017 at 2:52 PM Hitesh Khamesra  
wrote:

Basically, thread/layer should not hold any resources while serializing the 
object or chunk.  We should be able to see this flow (ms1-chunk1, msg2-chunk1, 
ms1-chunk2, msg3-chunk, msg2-chunk2, so on ...)


Correct, but putting that in the message layer is not appropriate. The simple 
solution is that the multiple channels can be achieved with multiple sockets. 
The later optimization is to add a channel multiplexer layer between the 
message and socket layers. 
If we put it in the message layer, not only does it for the message to tackle 
something it shouldn't be concerned with, reassembling itself, but it also 
forces all implementors to tackle this logic up front. By layering we can 
release without, implementors aren't forced into understanding the logic, and 
later we can release the layers and the client can negotiate.
 
On other pdx note: to de-serialize the pdx we need length of serialized bytes, 
so that we can read field offset from serialized stream, and then can read 
field value. Though, I can imagine with the help of pdxType, we can interpret 
serialized stream.


Yes, so today PDX serialization would be no worse, the PDX serializer would 
have to buffer, but other may not have to. The length of the buffered PDX could 
be used as the first chunk length and complete in single chunk. Although, I 
suspect that amortized overhead of splitting the chunks  will be nil anyway. 
The point is that the message encoding of values should NOT have any unbounded 
length fields and require long or many buffers to complete serialization. By 
chunking you can accomplish this by not needing to buffer the whole stream, 
just small (say 1k), chunks at a time to get the chunk length. 
Buffers == Latency
-Jake


   

Re: Review Request 59057: GEODE-2193 a member is kicked out immediately after joining

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59057/#review174198
---




geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
Line 830 (original)
<https://reviews.apache.org/r/59057/#comment247321>

I think problem here is, we send shutdown message using Tcp layer. In that 
case, "receiver1" gets that shutdown message and pass that info to membership 
layer. Then "receiver1" becomes coordinator(legal coordinator) by removing 
current coordinator. Now if current coordinator sends new view then cluster 
just ignores that view, as cluster has new-view by "receiver1".


- Hitesh Khamesra


On May 8, 2017, 5:23 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59057/
> ---
> 
> (Updated May 8, 2017, 5:23 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2193
> https://issues.apache.org/jira/browse/GEODE-2193
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The previous fix for this ticket introduced a shutdown problem that caused 
> servers to pause waiting for ShutdownMessage to be sent to another server 
> that had already exited.  We reduced the pause time but this change set fixes 
> the problem by transmitting the message over UDP instead of TCP/IP stream 
> sockets.
> 
> Another change in GMSJoinLeave prepareView/sendView allows a membership 
> coordinator that is shutting down to complete the sending out of a new view 
> if it has already prepared the view when shutdown begins.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  e0c0ba29a5c74614d2430fb78d972e306a355845 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  8ae66d0b6839cfbd46b479d896104f54fd11a68d 
>   geode-core/src/main/java/org/apache/geode/internal/util/PluckStacks.java 
> 357812a6ec0cb09a88fa727a4bf828f18794264d 
> 
> 
> Diff: https://reviews.apache.org/r/59057/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin plus 1000 runs of the test that was hitting this issue at least 4% 
> of the time
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 59057: GEODE-2193 a member is kicked out immediately after joining

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59057/#review174199
---




geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
Line 1865 (original), 1865 (patched)
<https://reviews.apache.org/r/59057/#comment247324>

I am not sure this will also helps as it is similar to real 
proplem(describe earlier), where receiver will become new coordinator. And that 
will create new view by removing current coordinator.


- Hitesh Khamesra


On May 8, 2017, 5:23 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59057/
> ---
> 
> (Updated May 8, 2017, 5:23 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2193
> https://issues.apache.org/jira/browse/GEODE-2193
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The previous fix for this ticket introduced a shutdown problem that caused 
> servers to pause waiting for ShutdownMessage to be sent to another server 
> that had already exited.  We reduced the pause time but this change set fixes 
> the problem by transmitting the message over UDP instead of TCP/IP stream 
> sockets.
> 
> Another change in GMSJoinLeave prepareView/sendView allows a membership 
> coordinator that is shutting down to complete the sending out of a new view 
> if it has already prepared the view when shutdown begins.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  e0c0ba29a5c74614d2430fb78d972e306a355845 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  8ae66d0b6839cfbd46b479d896104f54fd11a68d 
>   geode-core/src/main/java/org/apache/geode/internal/util/PluckStacks.java 
> 357812a6ec0cb09a88fa727a4bf828f18794264d 
> 
> 
> Diff: https://reviews.apache.org/r/59057/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin plus 1000 runs of the test that was hitting this issue at least 4% 
> of the time
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 59057: GEODE-2193 a member is kicked out immediately after joining

[Hitesh Khamesra]

> On May 8, 2017, 5:44 p.m., Hitesh Khamesra wrote:
> >

How about sending pending joinRequest(new member) with shutdown message. And 
let new coordinator take care of it.


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59057/#review174198
---


On May 8, 2017, 5:23 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59057/
> ---
> 
> (Updated May 8, 2017, 5:23 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2193
> https://issues.apache.org/jira/browse/GEODE-2193
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The previous fix for this ticket introduced a shutdown problem that caused 
> servers to pause waiting for ShutdownMessage to be sent to another server 
> that had already exited.  We reduced the pause time but this change set fixes 
> the problem by transmitting the message over UDP instead of TCP/IP stream 
> sockets.
> 
> Another change in GMSJoinLeave prepareView/sendView allows a membership 
> coordinator that is shutting down to complete the sending out of a new view 
> if it has already prepared the view when shutdown begins.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  e0c0ba29a5c74614d2430fb78d972e306a355845 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  8ae66d0b6839cfbd46b479d896104f54fd11a68d 
>   geode-core/src/main/java/org/apache/geode/internal/util/PluckStacks.java 
> 357812a6ec0cb09a88fa727a4bf828f18794264d 
> 
> 
> Diff: https://reviews.apache.org/r/59057/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin plus 1000 runs of the test that was hitting this issue at least 4% 
> of the time
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 59034: GEODE-2352 Document that REST API requires two properties

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59034/#review174224
---




geode-docs/rest_apps/setup_config.html.md.erb
Lines 84 (patched)
<https://reviews.apache.org/r/59034/#comment247338>

This looks good to me. ship it


- Hitesh Khamesra


On May 5, 2017, 10:32 p.m., Dave Barnes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59034/
> ---
> 
> (Updated May 5, 2017, 10:32 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Joey McAllister, and Pulkit 
> Chandra.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2352 Document that REST API requires two properties
> 
> 
> Diffs
> -
> 
>   geode-docs/rest_apps/chapter_overview.html.md.erb 
> 5e48675b7eba37ae3f6cd078111c0acf09d9ae68 
>   geode-docs/rest_apps/rest_prereqs.html.md.erb 
> 935f94b2caf83478bd0ac97dc797ccdcee695b8c 
>   geode-docs/rest_apps/setup_config.html.md.erb 
> 71c50f53dbc080f9d973b268eade9cb03c05e746 
> 
> 
> Diff: https://reviews.apache.org/r/59034/diff/1/
> 
> 
> Testing
> ---
> 
> The two properties, `http-service-bind-address` and `http-service-port`, are 
> not *mandatory* as stated in the ticket, but if allowed to default, they may 
> not be publicly visible. So we'll say they're optional, but that it's *good 
> practice* to specify them for visibility.
> This checkin also picks up some other corrections regarding API library name 
> and some format fixes.
> 
> 
> Thanks,
> 
> Dave Barnes
> 
>

Re: Review Request 59071: GEODE-2875 shutdown is taking as long as 20 seconds

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59071/#review174234
---


Ship it!




Ship It!

- Hitesh Khamesra


On May 8, 2017, 10:07 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59071/
> ---
> 
> (Updated May 8, 2017, 10:07 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2875
> https://issues.apache.org/jira/browse/GEODE-2875
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The band-aid fix for this problem was to reduce the wait-time on joining a 
> thread sending shutdown messages.
> 
> This change set alters the membership manager, reviving the path of sending 
> certain messages like ShutdownMessage over UDP instead of TCP/IP stream 
> sockets.  This avenue doesn't block trying to form point-to-point connections 
> so the join() can complete in a short amount of time.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  e0c0ba29a5c74614d2430fb78d972e306a355845 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  8ae66d0b6839cfbd46b479d896104f54fd11a68d 
> 
> 
> Diff: https://reviews.apache.org/r/59071/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 59237: GEODE-2875 shutdown is taking as long as 20 seconds

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59237/#review174831
---


Ship it!




Ship It!

- Hitesh Khamesra


On May 12, 2017, 6:29 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59237/
> ---
> 
> (Updated May 12, 2017, 6:29 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2875
> https://issues.apache.org/jira/browse/GEODE-2875
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The fix for this issue causes one of the test cases in LocatorDUnitTest to 
> fail consistently. With the fix we don't create any TCP/IP connections in 
> this test during startup but the test expects one to have been created and it 
> expects the connection's reader-thread to have initiated suspect processing.
> 
> The test needs to be altered to ensure that this thread has been created by 
> sending message that requires a reply. The reply will be sent over the 
> expected connection, ensuring that there is a reader-thread.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 6f01f9094bb9cfb35031c51472b2fd9a532e3ee3 
> 
> 
> Diff: https://reviews.apache.org/r/59237/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Review Request 59239: Allow a locator host to be taken off line and replaced with a different machine having the same host name

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59239/
---

Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo Kohlmeyer.


Repository: geode


Description
---

GEODE-2804 Update InetSocketAddress, when there is IOException.

Geode keeps InetSocketAddress for locators. So, if locators ip
changes in cloud/VM enviroment then Geode process unable to
connect to locator. Thus we have fixed this problem in two way.

a. If Geode client sees IOException while connectin to locator then
we change cached InetAddress to use locator hostname. In this way,
client does DNS query again for locator host.
b. For other Geode process, now we connect to locator using hostname.

Added couple of junit tests for it.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 1e807ee 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 4bf010b 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 6b54170 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 0efba01 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 b3de975 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 385569c 


Diff: https://reviews.apache.org/r/59239/diff/1/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 59239: Allow a locator host to be taken off line and replaced with a different machine having the same host name

[Hitesh Khamesra]

> On May 12, 2017, 9:53 p.m., Bruce Schuchardt wrote:
> > It would be nice to have a javadoc on the two tests.  Other than that the 
> > changes look good to me Hitesh.

I will update that. Thanks.


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59239/#review174861
---


On May 12, 2017, 7:01 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59239/
> ---
> 
> (Updated May 12, 2017, 7:01 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2804 Update InetSocketAddress, when there is IOException.
> 
> Geode keeps InetSocketAddress for locators. So, if locators ip
> changes in cloud/VM enviroment then Geode process unable to
> connect to locator. Thus we have fixed this problem in two way.
> 
> a. If Geode client sees IOException while connectin to locator then
> we change cached InetAddress to use locator hostname. In this way,
> client does DNS query again for locator host.
> b. For other Geode process, now we connect to locator using hostname.
> 
> Added couple of junit tests for it.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  1e807ee 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  4bf010b 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  6b54170 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
>  0efba01 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  b3de975 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  385569c 
> 
> 
> Diff: https://reviews.apache.org/r/59239/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 59239: Allow a locator host to be taken off line and replaced with a different machine having the same host name

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59239/
---

(Updated May 15, 2017, 6:33 p.m.)


Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo Kohlmeyer.


Changes
---

Updated comments on test


Repository: geode


Description
---

GEODE-2804 Update InetSocketAddress, when there is IOException.

Geode keeps InetSocketAddress for locators. So, if locators ip
changes in cloud/VM enviroment then Geode process unable to
connect to locator. Thus we have fixed this problem in two way.

a. If Geode client sees IOException while connectin to locator then
we change cached InetAddress to use locator hostname. In this way,
client does DNS query again for locator host.
b. For other Geode process, now we connect to locator using hostname.

Added couple of junit tests for it.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 1e807ee 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 4bf010b 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 6b54170 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 0efba01 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 d61e72d 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 385569c 


Diff: https://reviews.apache.org/r/59239/diff/2/

Changes: https://reviews.apache.org/r/59239/diff/1-2/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 59242: GEODE-2915 Messages rejected due to unknown "vmkind"

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59242/#review174998
---


Ship it!




Ship It!

- Hitesh Khamesra


On May 12, 2017, 8:28 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59242/
> ---
> 
> (Updated May 12, 2017, 8:28 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2915
> https://issues.apache.org/jira/browse/GEODE-2915
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The fix for GEODE_2875 has exacerbated this problem, which we used to only 
> see in cases where disable-tcp=true or when multicast was enabled.
> 
> The problem is that JGroupsMessenger is not sending the "vmkind" of the 
> sender in message headers.  This part of the header comes from 
> GMSMember.writeEssentialData().  I've changed it here to include the vmKind 
> if the recipient isn't using geode 1.0, which doesn't expect the version byte.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
>  41c85d6421c8283163b70f2a560c8e4cbb02f2cc 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java
>  b7079f8bc20a0e58949b69b9f0174a26af1a9b86 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  0476bbbfa4a1480d3b31a052e98dc62d9f0e3867 
>   
> geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
> 85042da54f5a2a772d39ba450110073e14a30196 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberJUnitTest.java
>  f471ad99b56615a1935ccf52127960f4af763d7d 
> 
> 
> Diff: https://reviews.apache.org/r/59242/diff/1/
> 
> 
> Testing
> ---
> 
> new unit test.  Precheckin is underway.  I expect AnalyzeSerializables to 
> fail & will need to update its sanctionedDataSerializables.txt record for 
> GMSMember.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 58937: GEODE-2865 data loss in initial-image replication with multicast

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58937/#review175012
---


Ship it!




Ship It!

- Hitesh Khamesra


On May 5, 2017, 4:57 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58937/
> ---
> 
> (Updated May 5, 2017, 4:57 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2865
> https://issues.apache.org/jira/browse/GEODE-2865
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The state-flush algorithm relies on MembershipManager.waitForMessageState() 
> to ensure that all operations have been received and applied to the cache 
> prior to state replication starting.  For multicast there was a flaw in the 
> algorithm caused by two things: 1) cache operations were being sent 
> out-of-band, allowing them to be processed out of order with the state-flush 
> message, and 2) JGroupsMessenger was only waiting for the messages to be 
> dispatched by NAKACK2, which isn't necessarily the same as being dispatched 
> to the DistributionManager Executor that processes the message.
> 
> Cache operation messages are now sent in-band.
> JGroupsMessenger now tracks NAKACK2 (multicast) sequence numbers of messages 
> dispatched to the DistributionManager and this is used in 
> waitForMessageState() to make sure the messages have been queued.
> If multicast is enabled we now flush the serial executor to in 
> waitForMessageState() to make sure that all messages queued in it have been 
> applied to the region.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  e99eff2be344d54da67c257a0bfa73ad8268c4c6 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  8ae66d0b6839cfbd46b479d896104f54fd11a68d 
>   
> geode-core/src/test/java/org/apache/geode/distributed/DistributedSystemDUnitTest.java
>  9a64f531431e714916765d6d6c741841ef719fb8 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessengerJUnitTest.java
>  307b5948c02befee61d61b628c38b8b8b8693c4d 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/FixedPRSinglehopDUnitTest.java
>  7e798c8358aaec070d3dd9d04c2486bd33a21d9e 
> 
> 
> Diff: https://reviews.apache.org/r/58937/diff/2/
> 
> 
> Testing
> ---
> 
> passes precheckin and modified unit tests
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 59239: Allow a locator host to be taken off line and replaced with a different machine having the same host name

[Hitesh Khamesra]

> On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote:
> > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
> > Lines 217 (patched)
> > <https://reviews.apache.org/r/59239/diff/2/?file=1718520#file1718520line219>
> >
> > The exception should be logged in `reportDeadLocator`. Although perhaps 
> > the message in there should be changed from "locator {0} is not running" to 
> > "could not connect to locator {0}".
> > 
> > If we keep the log statement, I think it's better to use the other form 
> > `logger.info("queryOneLocator got Exception ", ioe);`

Good Catch. I have removed that log, otherwise it will fill the log file


> On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote:
> > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
> > Lines 237 (patched)
> > <https://reviews.apache.org/r/59239/diff/2/?file=1718520#file1718520line239>
> >
> > If connecting to the locator fails with an `IOException`, this may be 
> > because the locator's IP has changed. Add the locator back to the list of 
> > locators using host address rather than IP. This will cause another DNS 
> > lookup, hopefully finding the locator.

Updated


> On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote:
> > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
> > Lines 248 (patched)
> > <https://reviews.apache.org/r/59239/diff/2/?file=1718520#file1718520line250>
> >
> > This could be written as 
> > 
> > `for (InetSocketAddress tloc : locatorList.getLocators())`
> > 
> > and `locIterator` declaration removed.

updated


> On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote:
> > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
> > Lines 250 (patched)
> > <https://reviews.apache.org/r/59239/diff/2/?file=1718520#file1718520line252>
> >
> > In the case where the locator isn't in the list, do we mean to add it 
> > to `newLocatorList` or discard it?
> > 
> > If we mean to add it, we should move the check `locator.getHostName() 
> > != null` to above the loop.

Not sure why I had added that but I put that check up.


> On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote:
> > geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
> > Lines 155 (patched)
> > <https://reviews.apache.org/r/59239/diff/2/?file=1718525#file1718525line155>
> >
> > Is floc2 necessary?

It just to test the logic.


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59239/#review175000
---


On May 15, 2017, 6:33 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59239/
> ---
> 
> (Updated May 15, 2017, 6:33 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2804 Update InetSocketAddress, when there is IOException.
> 
> Geode keeps InetSocketAddress for locators. So, if locators ip
> changes in cloud/VM enviroment then Geode process unable to
> connect to locator. Thus we have fixed this problem in two way.
> 
> a. If Geode client sees IOException while connectin to locator then
> we change cached InetAddress to use locator hostname. In this way,
> client does DNS query again for locator host.
> b. For other Geode process, now we connect to locator using hostname.
> 
> Added couple of junit tests for it.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  1e807ee 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  4bf010b 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  6b54170 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
>  0efba01 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  d61e72d 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  385569c 
> 
> 
> Diff: https://reviews.apache.org/r/59239/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 59239: Allow a locator host to be taken off line and replaced with a different machine having the same host name

[Hitesh Khamesra]

> On May 15, 2017, 6:45 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
> > Line 269 (original), 308 (patched)
> > <https://reviews.apache.org/r/59239/diff/1/?file=1717126#file1717126line310>
> >
> > It might be just me, but I don't see how the `initialLocators` list is 
> > suddenly bad. We should just treat it as an `initialList`. Why would they 
> > suddenly be `bad`?

We got new locator list from live locator and it doesn't contain the initial 
locator.


> On May 15, 2017, 6:45 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
> > Lines 351 (patched)
> > <https://reviews.apache.org/r/59239/diff/1/?file=1717126#file1717126line353>
> >
> > What makes a locator "bad"? I would argue that we make this method more 
> > of a general "mergeLocatorLists". The fact that we add list containing 
> > "bad" locators to another list containing locators, is circumstantial.

We could change name of that method but I think it is giving you intent of that 
method explicitly. And thats how its define in code.


> On May 15, 2017, 6:45 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
> > Lines 168 (patched)
> > <https://reviews.apache.org/r/59239/diff/1/?file=1717131#file1717131line168>
> >
> > Maybe this becomes a test that we can successfully merge 2 locator 
> > lists.

We just validaing method logic.


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59239/#review174980
---


On May 15, 2017, 6:33 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59239/
> ---
> 
> (Updated May 15, 2017, 6:33 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2804 Update InetSocketAddress, when there is IOException.
> 
> Geode keeps InetSocketAddress for locators. So, if locators ip
> changes in cloud/VM enviroment then Geode process unable to
> connect to locator. Thus we have fixed this problem in two way.
> 
> a. If Geode client sees IOException while connectin to locator then
> we change cached InetAddress to use locator hostname. In this way,
> client does DNS query again for locator host.
> b. For other Geode process, now we connect to locator using hostname.
> 
> Added couple of junit tests for it.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  1e807ee 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  4bf010b 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  6b54170 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
>  0efba01 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  d61e72d 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  385569c 
> 
> 
> Diff: https://reviews.apache.org/r/59239/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 59239: Allow a locator host to be taken off line and replaced with a different machine having the same host name

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59239/
---

(Updated May 15, 2017, 8:46 p.m.)


Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo Kohlmeyer.


Changes
---

Updated the review comment


Repository: geode


Description
---

GEODE-2804 Update InetSocketAddress, when there is IOException.

Geode keeps InetSocketAddress for locators. So, if locators ip
changes in cloud/VM enviroment then Geode process unable to
connect to locator. Thus we have fixed this problem in two way.

a. If Geode client sees IOException while connectin to locator then
we change cached InetAddress to use locator hostname. In this way,
client does DNS query again for locator host.
b. For other Geode process, now we connect to locator using hostname.

Added couple of junit tests for it.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 1e807ee 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 4bf010b 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 6b54170 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 0efba01 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 d61e72d 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 385569c 


Diff: https://reviews.apache.org/r/59239/diff/3/

Changes: https://reviews.apache.org/r/59239/diff/2-3/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 59239: Allow a locator host to be taken off line and replaced with a different machine having the same host name

[Hitesh Khamesra]

> On May 15, 2017, 10:10 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
> > Lines 148 (patched)
> > <https://reviews.apache.org/r/59239/diff/3/?file=1719293#file1719293line148>
> >
> > It isn't immediately evident why `floc1` is not equal to the `floc1` 
> > that is updated in `src.updateLocatorInLocatorList`. 
> > Also maybe a `==` instance comparison is better than identityHashCode

I will change that. Thanks Bruce and Udo.


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59239/#review175022
-------


On May 15, 2017, 8:46 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59239/
> ---
> 
> (Updated May 15, 2017, 8:46 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2804 Update InetSocketAddress, when there is IOException.
> 
> Geode keeps InetSocketAddress for locators. So, if locators ip
> changes in cloud/VM enviroment then Geode process unable to
> connect to locator. Thus we have fixed this problem in two way.
> 
> a. If Geode client sees IOException while connectin to locator then
> we change cached InetAddress to use locator hostname. In this way,
> client does DNS query again for locator host.
> b. For other Geode process, now we connect to locator using hostname.
> 
> Added couple of junit tests for it.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  1e807ee 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  4bf010b 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  6b54170 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
>  0efba01 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  d61e72d 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  385569c 
> 
> 
> Diff: https://reviews.apache.org/r/59239/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 59242: GEODE-2915 Messages rejected due to unknown "vmkind"

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59242/#review175149
---


Ship it!




Ship It!

- Hitesh Khamesra


On May 15, 2017, 11:13 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59242/
> ---
> 
> (Updated May 15, 2017, 11:13 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2915
> https://issues.apache.org/jira/browse/GEODE-2915
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The fix for GEODE_2875 has exacerbated this problem, which we used to only 
> see in cases where disable-tcp=true or when multicast was enabled.
> 
> The problem is that JGroupsMessenger is not sending the "vmkind" of the 
> sender in message headers.  This part of the header comes from 
> GMSMember.writeEssentialData().  I've changed it here to include the vmKind 
> if the recipient isn't using geode 1.0, which doesn't expect the version byte.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
>  41c85d6421c8283163b70f2a560c8e4cbb02f2cc 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java
>  b7079f8bc20a0e58949b69b9f0174a26af1a9b86 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  bfc8b61ff9e9c49568a0c6e19381714ea8fbba05 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  8cdd6a58df82a4e11cd1c2f864650a3da20aaec6 
>   geode-core/src/main/java/org/apache/geode/internal/Version.java 
> 288d1049eb5fe2134e485e4d89a2538b2d5115f4 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
>  71586a0f1866e63c6314a6884f144c9342aace4b 
>   
> geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
> 1b33094c9b337db3d1b65ec6132819b867d841cf 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java
>  f740dde3083e78f2df19ecdd5445ac6e6e013057 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberJUnitTest.java
>  f471ad99b56615a1935ccf52127960f4af763d7d 
>   geode-old-versions/build.gradle eb82a5ffb950826b9fa0072ea5ba0f3a505aa010 
> 
> 
> Diff: https://reviews.apache.org/r/59242/diff/2/
> 
> 
> Testing
> ---
> 
> new unit test.  Precheckin is underway.  I expect AnalyzeSerializables to 
> fail & will need to update its sanctionedDataSerializables.txt record for 
> GMSMember.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 59422: GEODE-2954 Old client gets null memberID in cache listener

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59422/#review175989
---


Ship it!




Ship It!

- Hitesh Khamesra


On May 19, 2017, 10:48 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59422/
> ---
> 
> (Updated May 19, 2017, 10:48 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2954
> https://issues.apache.org/jira/browse/GEODE-2954
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> I've added a new test that demonstrates that a new-version server sends an 
> EventID to a client that the client is unable to deserialize completely.  It 
> gets an error when deserializing its member ID, causing cache listeners to 
> get a null when requesting the ID of the member that effected the change.
> 
> The fix is to reserialize the member ID in EventID.toData if the destination 
> stream is for an older version, such as a 1.1.0 client.  This ensures the 
> proper on-wire format is used for that version of Geode.
> 
> I've also bumped up the version ordinal for 1.2 since version 59 is marked as 
> unusable in Version.java.
> 
> I've changed the Banner to show the version ordinal because the other version 
> information in the banner isn't completely trustworthy.  It looks for a 
> GemFireVersion.properties file on the classpath to get this information and 
> so it may not get it from the Geode jar file as expected.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/Banner.java 
> b6a89bfc530fa5f4766f61e124839479dff2b664 
>   geode-core/src/main/java/org/apache/geode/internal/Version.java 
> 1c131e8d08fc4b3f8004ffaca78fb6fac910ee2b 
>   geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java 
> 87835ffa5a9782fecec5f6ae7adfe9829ac2fc26 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
>  be0ac6b080652179f12ccaf0e0a14f7acc299269 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscDUnitTest.java
>  b4f3185ea71e47fc32c8ef1b3e656f4026056526 
> 
> 
> Diff: https://reviews.apache.org/r/59422/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin, new unit test.  I have to fix up the database for 
> AnalyzeSerializablesJUnitTest - I'm not including that in this review's diff.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Review Request 59546: GEODE-2940 Remove verification of locator host on start

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59546/
---

Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo Kohlmeyer.


Repository: geode


Description
---

Patch from Bruce. Modified couple of tests.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
 c1bfc93 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 01c6157 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 7caad3f 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
 5ab1bed 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
 1dc2fd1 
  
geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java
 dc73f04 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 9f6c5fb 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 d6d5d7c 


Diff: https://reviews.apache.org/r/59546/diff/1/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 59546: GEODE-2940 Remove verification of locator host on start

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59546/
---

(Updated May 25, 2017, 5:12 p.m.)


Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo Kohlmeyer.


Repository: geode


Description (updated)
---

We configure locator list to start the cache. This locator list is validated 
while creating the cache. We verify whether locator host exist or not. Now we 
have remove this verification as in cloud environment host may not available 
for time being. 

Patch from Bruce. Modified couple of tests.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
 c1bfc93 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 01c6157 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 7caad3f 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
 5ab1bed 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
 1dc2fd1 
  
geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java
 dc73f04 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 9f6c5fb 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 d6d5d7c 


Diff: https://reviews.apache.org/r/59546/diff/1/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 59546: GEODE-2940 Remove verification of locator host on start

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59546/
---

(Updated May 25, 2017, 7:06 p.m.)


Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo Kohlmeyer.


Changes
---

Updated diff; removed commented code


Repository: geode


Description
---

We configure locator list to start the cache. This locator list is validated 
while creating the cache. We verify whether locator host exist or not. Now we 
have remove this verification as in cloud environment host may not available 
for time being. 

Patch from Bruce. Modified couple of tests.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
 c1bfc93 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 01c6157 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 7caad3f 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
 5ab1bed 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
 1dc2fd1 
  
geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java
 dc73f04 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 9f6c5fb 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 d6d5d7c 


Diff: https://reviews.apache.org/r/59546/diff/2/

Changes: https://reviews.apache.org/r/59546/diff/1-2/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 59546: GEODE-2940 Remove verification of locator host on start

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59546/#review176113
---




geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
Line 196 (original), 196 (patched)
<https://reviews.apache.org/r/59546/#comment249446>

Removed it. here we validate hist only



geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
Line 277 (original), 281 (patched)
<https://reviews.apache.org/r/59546/#comment249447>

Removed commented code



geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
Line 1555 (original), 1555 (patched)
<https://reviews.apache.org/r/59546/#comment249448>

This is just for compare purpose.



geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
Line 131 (original), 132 (patched)
<https://reviews.apache.org/r/59546/#comment249449>

Yes, that's the change.Removed commented code



geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
Lines 234 (patched)
<https://reviews.apache.org/r/59546/#comment249453>

line 131; but then hostname will be there. What I am reading is that at 
this point either host/hostname will be there



geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
Lines 278 (patched)
<https://reviews.apache.org/r/59546/#comment249451>

Ip should be fine. This can throw exception when host/hostname is not 
appropriate.



geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
Lines 346 (patched)
<https://reviews.apache.org/r/59546/#comment249452>

If locator host is still not avialable then server won't be able to join 
the cluster.


- Hitesh Khamesra


On May 25, 2017, 7:06 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59546/
> ---
> 
> (Updated May 25, 2017, 7:06 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> We configure locator list to start the cache. This locator list is validated 
> while creating the cache. We verify whether locator host exist or not. Now we 
> have remove this verification as in cloud environment host may not available 
> for time being. 
> 
> Patch from Bruce. Modified couple of tests.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
>  c1bfc93 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
>  01c6157 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  7caad3f 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
>  5ab1bed 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
>  1dc2fd1 
>   
> geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java
>  dc73f04 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
>  9f6c5fb 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d 
>   
> geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
>  d6d5d7c 
> 
> 
> Diff: https://reviews.apache.org/r/59546/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 59819: GEODE-3034 java.lang.ArrayIndexOutOfBoundsException: 0 on auto-reconnect attempt with multicast enabled

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59819/#review177091
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 5, 2017, 10:14 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59819/
> ---
> 
> (Updated June 5, 2017, 10:14 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-3034
> https://issues.apache.org/jira/browse/GEODE-3034
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> A bug in JGroups causes this exception.  A workaround is to add a non-usable 
> UUID-based address to the view that we use to reinialize JGroups during an 
> auto-reconnect attempt.  We've sent this issue to the JGroups email list.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  b07aa591ccc4a559f4f8801fae3809a670dd359c 
> 
> 
> Diff: https://reviews.apache.org/r/59819/diff/2/
> 
> 
> Testing
> ---
> 
> Since this involves multicast we can't create a unit test to verify the fix, 
> though we did so on a machine with mcast support to make sure the fix is 
> correct.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 59757: GEODE-3024 race condition between server and restarted locator preparing membership views

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59757/#review177093
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 2, 2017, 7:18 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59757/
> ---
> 
> (Updated June 2, 2017, 7:18 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3024
> https://issues.apache.org/jira/browse/GEODE-3024
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> If a locator is preparing a conflicting membership view we now abandon 
> preparation of a view in a cache server and pause before retrying.  This 
> gives the locator time to gather information from the cache server's view 
> (which it receives in acks while preparing its own view), incorporate them 
> into a new view and send it out.  When the cache server installs the new view 
> from the locator it will shut down its ViewCreator thread.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  478c4e832dd18635d0d66c4b85e2e2ce6ed5ab91 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messages/ViewAckMessage.java
>  71bd6bcbf60fbcd9e6320195c5d9a5e1cf809e8a 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
>  49b09cab058e9e68c0e029b048e9c03c585b7fe5 
> 
> 
> Diff: https://reviews.apache.org/r/59757/diff/1/
> 
> 
> Testing
> ---
> 
> new unit test, precheckin, partial regression testing
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 59863: Removing obsolete SSL handling in `AcceptorImpl.accept` catch block

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59863/#review177253
---




geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
Line 1252 (original)
<https://reviews.apache.org/r/59863/#comment250791>

Do we need to move this code in another thread?


- Hitesh Khamesra


On June 6, 2017, 10:19 p.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59863/
> ---
> 
> (Updated June 6, 2017, 10:19 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> SSL handshake is now done in a separate thread and will never reach the 
> handler code which is being removed.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  9658f98da 
> 
> 
> Diff: https://reviews.apache.org/r/59863/diff/1/
> 
> 
> Testing
> ---
> 
> Precheckin started
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>

Re: Review Request 59850: GEODE-3023: TcpServer thread can be blocked in processRequest

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59850/#review177256
---


Fix it, then Ship it!




Ship It!


geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
Line 171 (original)
<https://reviews.apache.org/r/59850/#comment250793>

Is that code moved inside/will never happen?


- Hitesh Khamesra


On June 7, 2017, 11:32 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59850/
> ---
> 
> (Updated June 7, 2017, 11:32 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Moved the socket.setSoTimeout setting to be before the SSL handshake. This is 
> to avoid the timeout to never be set in the case of a SSLException. Added a 
> test to test that the socket timeout is correctly set upon failure within the 
> SSL configuration and handshake.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
>  86fe53261 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TcpServerJUnitTest.java
>  7c7a2b376 
>   
> geode-core/src/test/java/org/apache/geode/internal/net/DelaySocketCreator.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59850/diff/2/
> 
> 
> Testing
> ---
> 
> Junit test - done
> precheckin - in progress
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Review Request 59924: Reverted GEODE-2804 (Allow a locator host to be taken off line and replaced with a different machine having the same host name)

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59924/
---

Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo Kohlmeyer.


Repository: geode


Description
---

There was conflict in "LauncherLifecycleCommands", this code moved to 
"ClusterConfigurationStatusRetriever".

We are reverting this issue as we cache InetAddress in geode. But caching 
"hostname" is causing various issuess.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 070451c 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 84d42cf 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 e9476b5 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 eb71d38 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 4c668b6 
  
geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/ClusterConfigurationStatusRetriever.java
 9f35edd 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 9ceb461 


Diff: https://reviews.apache.org/r/59924/diff/1/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 59894: GEODE-3041 CI failure: DistributedMemberDUnitTest.testGroupsInAllVMs fails intermittently

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59894/#review177350
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 7, 2017, 9:56 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59894/
> ---
> 
> (Updated June 7, 2017, 9:56 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Bugs: GEODE-3041
> https://issues.apache.org/jira/browse/GEODE-3041
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When shutting down the MembershipManager after it has joined we should not 
> use uncleanShutdown because the member will appear to have crashed.  Instead 
> we should do a normal shutdown.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionChannel.java
>  ef4056ca56c1b102e507f96bbfe41396d0139aa5 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  029e6377f56d80dd81e4cec430f106ac743e5178 
> 
> 
> Diff: https://reviews.apache.org/r/59894/diff/1/
> 
> 
> Testing
> ---
> 
> The test was failing every time I ran it.  It no longer fails.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 60217: GEODE-2995: adding in protocol handler for protobuf messages

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60217/#review178513
---




geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/EncodingTypeTranslator.java
Lines 56 (patched)
<https://reviews.apache.org/r/60217/#comment252537>

How about having map of "class vs Id"?



geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BooleanCodec.java
Lines 29 (patched)
<https://reviews.apache.org/r/60217/#comment252540>

How about keeping two byte[] here. One for true and one for false.



geode-protobuf/src/main/java/org/apache/geode/serialization/codec/ByteCodec.java
Lines 24 (patched)
<https://reviews.apache.org/r/60217/#comment252541>

Just return first byte?


- Hitesh Khamesra


On June 21, 2017, 12:02 a.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60217/
> ---
> 
> (Updated June 21, 2017, 12:02 a.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This change adds a new module for handling client stresms encoded using the 
> new ProtoBuf protocol.  At the top level this can be integrated by passing in 
> the input/output streams and cache reference to the ProtobufStreamProcessor.  
> This will decode the message and ultimately dispatch it to an operation 
> specific handler which will call back into the passed cache object.  Note 
> that this not currently hooked up to geode at all, GEODE-3075 is tracking the 
> work needed to hook this up at that level.
> 
> This change mainly contains the plumbing and encoding/decoding logic, but 
> also contain the Get operation handler as a proof of concept.  Future work 
> will be needed to handle other types of operations.
> 
> 
> Diffs
> -
> 
>   geode-protobuf/build.gradle PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/exception/InvalidProtocolMessageException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationHandler.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/OperationsHandlerRegistry.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerAlreadyRegisteredException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerNotRegisteredException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/EncodingTypeTranslator.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufSerializationService.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufUtilities.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/serializer/ProtobufProtocolSerializer.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/serializer/ProtocolSerializer.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationService.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationType.java
>  PRE-CREATION 
>   geode-protobuf/src/main/java/org/apache/geode/serialization/TypeCodec.java 
> PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BinaryCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BooleanCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/ByteCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/DoubleCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/FloatCodec.java
>  PRE-CREATION 
>   

Re: Review Request 60217: GEODE-2995: adding in protocol handler for protobuf messages

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60217/#review178527
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 21, 2017, 7:01 p.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60217/
> ---
> 
> (Updated June 21, 2017, 7:01 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This change adds a new module for handling client stresms encoded using the 
> new ProtoBuf protocol.  At the top level this can be integrated by passing in 
> the input/output streams and cache reference to the ProtobufStreamProcessor.  
> This will decode the message and ultimately dispatch it to an operation 
> specific handler which will call back into the passed cache object.  Note 
> that this not currently hooked up to geode at all, GEODE-3075 is tracking the 
> work needed to hook this up at that level.
> 
> This change mainly contains the plumbing and encoding/decoding logic, but 
> also contain the Get operation handler as a proof of concept.  Future work 
> will be needed to handle other types of operations.
> 
> 
> Diffs
> -
> 
>   geode-protobuf/build.gradle PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/exception/InvalidProtocolMessageException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationHandler.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/OperationsHandlerRegistry.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerAlreadyRegisteredException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerNotRegisteredException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/EncodingTypeTranslator.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufSerializationService.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufUtilities.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/serializer/ProtobufProtocolSerializer.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/serializer/ProtocolSerializer.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationService.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationType.java
>  PRE-CREATION 
>   geode-protobuf/src/main/java/org/apache/geode/serialization/TypeCodec.java 
> PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BinaryCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BooleanCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/ByteCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/DoubleCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/FloatCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/IntCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/JSONCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/LongCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/ShortCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/codec/StringCodec.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serializat

Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60312/
---

Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
O'Sullivan.


Repository: geode


Description
---

GEODE-2804 Cache InetAddress if configure host as ip string.

1. We keep configure host string in HostAddress class
2. We reuse InetsocketAddress if it is ipString.
3. Client has logic to retry thus we keep InetsocketAddress even if 
   it is not ip string.

GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.

GEODE-2940 Now we don't validate configure host string on start. As
there is possibility that host may not available.

1. Earlier wan config were failing because of that. Now we just keep
those configure host string. And try this later.

Added unit tests for it.


Diffs
-

  geode-assembly/build.gradle 39bb542 
  geode-assembly/src/test/resources/expected_jars.txt 6260167 
  geode-core/build.gradle 7f34b4a 
  
geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
 c1bfc93 
  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 070451c 
  geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
3ded54a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 01c6157 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 1572355 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
 c6bef57 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 93fa9da 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 84d42cf 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 e9476b5 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
 5ab1bed 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
 1dc2fd1 
  
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 4f4881f 
  geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
d4fdbd0 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
 789d326 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 9169904 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 9f6c5fb 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java
 9d63050 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
 a31fa8d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
6a6e0c1 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorDiscovery.java
 f5a8fcf 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 d6d5d7c 
  
geode-wan/src/main/java/org/apache/geode/internal/cache/wan/AbstractRemoteGatewaySender.java
 dbc2cc6 
  
geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/WanAutoDiscoveryDUnitTest.java
 6d75064 
  gradle/dependency-versions.properties 183dafc 


Diff: https://reviews.apache.org/r/60312/diff/1/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 60106: GEODE-3052 Restarting 2 locators within 1s of each other causes potential locator split brain

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60106/#review178573
---




geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
Lines 235 (patched)
<https://reviews.apache.org/r/60106/#comment252637>

we are setting -1 viewid for recorded view while recovery. Thus do we need 
"usingRecoveredView" flag here?


- Hitesh Khamesra


On June 19, 2017, 4:09 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60106/
> ---
> 
> (Updated June 19, 2017, 4:09 p.m.)
> 
> 
> Review request for geode and Hitesh Khamesra.
> 
> 
> Bugs: GEODE-3052
> https://issues.apache.org/jira/browse/GEODE-3052
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There were four problems that new unit tests hit:
> 1. when recovering a view from disk we were treating it as a definitive 
> (live) view.  I've moved it to a new variable in GMSLocator and set its 
> viewId to -1.  At the same time I set the initial GMSJoinLeave 
> SearchState.viewId to -100 so it will be overridden by the one returned by 
> the locator.  These changes allow GmsJoinLeave to know that the potential 
> coordinator is from a recovered view.
> 2. when trying to join with a recovered view GMSJoinLeave.join() was giving 
> up after the second ID in the view and becoming the coordinator.  It needs to 
> keep trying until the list is exhausted, and it shouldn't sleep between 
> attempts.
> 3. GMSLocator wasn't returning registrants for use in 
> findCoordinatorFromView().  This was causing it to choose itself as the 
> coordinator instead of using registrant sort order and choosing a different 
> registrant as the coordinator.
> 4. During concurrent startup GMSLocator didn't know when the decision was 
> made to become coordinator.  It is now notified of this decision and 
> processRequest() uses this flag to have it override anything in the 
> registrants set or in the recovered view.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
>  26b03276b0abbf6210a5602a8c551abe38edc261 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  c6bef571134c6444a297cc8fe0bb0b7eb95f41f4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java
>  c5fdf45411581a36feca220e14a0551f3197d368 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
>  edfaf625e6c652f46d9323c1116791f1c69fda59 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  93fa9dab4ec2c8e43fc41cfd3b8ad986f96cf00f 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  8abcc456e42ad00a558a93f87bd3ae74ce88d146 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java
>  c7b1a26b47cf2c913d9de30d6934ad5b3ac49840 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  390824eb11a2e72a21d951539c2e03ed8025be82 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 7ecca6146f6b7a542ae9864d7fabd48c9794ecac 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
>  df1d8d1101a5f9d04c402922955a283353aa3b7c 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
>  19cee066a488198471ebf4093045853e36d5ba78 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java
>  7f64c670400464aa8e6a73405516bd6e891a006b 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java
>  b35270e2d97930cee68d8c54221a04c20dfb96de 
> 
> 
> Diff: https://reviews.apache.org/r/60106/diff/2/
> 
> 
> Testing
> ---
> 
> New unit tests, regression testing (under way), precheckin (under way)
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 60106: GEODE-3052 Restarting 2 locators within 1s of each other causes potential locator split brain

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60106/#review178585
---




geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
Lines 710 (patched)
<https://reviews.apache.org/r/60106/#comment252662>

In which condition we need to initialize cluster key here?



geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
Lines 800 (patched)
<https://reviews.apache.org/r/60106/#comment252665>

This looks good one!!



geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java
Lines 1748 (patched)
<https://reviews.apache.org/r/60106/#comment252668>

Very good test Bruce.



geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java
Line 79 (original), 79 (patched)
<https://reviews.apache.org/r/60106/#comment252666>

do we want fine level log here?


- Hitesh Khamesra


On June 21, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60106/
> ---
> 
> (Updated June 21, 2017, 10:24 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Brian Rowe.
> 
> 
> Bugs: GEODE-3052
> https://issues.apache.org/jira/browse/GEODE-3052
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There were four problems that new unit tests hit:
> 1. when recovering a view from disk we were treating it as a definitive 
> (live) view.  I've moved it to a new variable in GMSLocator and set its 
> viewId to -1.  At the same time I set the initial GMSJoinLeave 
> SearchState.viewId to -100 so it will be overridden by the one returned by 
> the locator.  These changes allow GmsJoinLeave to know that the potential 
> coordinator is from a recovered view.
> 2. when trying to join with a recovered view GMSJoinLeave.join() was giving 
> up after the second ID in the view and becoming the coordinator.  It needs to 
> keep trying until the list is exhausted, and it shouldn't sleep between 
> attempts.
> 3. GMSLocator wasn't returning registrants for use in 
> findCoordinatorFromView().  This was causing it to choose itself as the 
> coordinator instead of using registrant sort order and choosing a different 
> registrant as the coordinator.
> 4. During concurrent startup GMSLocator didn't know when the decision was 
> made to become coordinator.  It is now notified of this decision and 
> processRequest() uses this flag to have it override anything in the 
> registrants set or in the recovered view.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
>  26b03276b0abbf6210a5602a8c551abe38edc261 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  c6bef571134c6444a297cc8fe0bb0b7eb95f41f4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java
>  c5fdf45411581a36feca220e14a0551f3197d368 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
>  edfaf625e6c652f46d9323c1116791f1c69fda59 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  93fa9dab4ec2c8e43fc41cfd3b8ad986f96cf00f 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  8abcc456e42ad00a558a93f87bd3ae74ce88d146 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java
>  c7b1a26b47cf2c913d9de30d6934ad5b3ac49840 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  390824eb11a2e72a21d951539c2e03ed8025be82 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 7ecca6146f6b7a542ae9864d7fabd48c9794ecac 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
>  df1d8d1101a5f9d04c402922955a283353aa3b7c 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
>  19cee066a488198471ebf4093045853e36d5ba78 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java
>  7f64

Re: Review Request 60106: GEODE-3052 Restarting 2 locators within 1s of each other causes potential locator split brain

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60106/#review178585
---




geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
Lines 710 (patched)
<https://reviews.apache.org/r/60106/#comment252662>

In which condition we need to initialize cluster key here?



geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
Lines 800 (patched)
<https://reviews.apache.org/r/60106/#comment252665>

This looks good one!!



geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java
Lines 1748 (patched)
<https://reviews.apache.org/r/60106/#comment252668>

Very good test Bruce.



geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java
Line 79 (original), 79 (patched)
<https://reviews.apache.org/r/60106/#comment252666>

do we want fine level log here?


- Hitesh Khamesra


On June 21, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60106/
> ---
> 
> (Updated June 21, 2017, 10:24 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Brian Rowe.
> 
> 
> Bugs: GEODE-3052
> https://issues.apache.org/jira/browse/GEODE-3052
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There were four problems that new unit tests hit:
> 1. when recovering a view from disk we were treating it as a definitive 
> (live) view.  I've moved it to a new variable in GMSLocator and set its 
> viewId to -1.  At the same time I set the initial GMSJoinLeave 
> SearchState.viewId to -100 so it will be overridden by the one returned by 
> the locator.  These changes allow GmsJoinLeave to know that the potential 
> coordinator is from a recovered view.
> 2. when trying to join with a recovered view GMSJoinLeave.join() was giving 
> up after the second ID in the view and becoming the coordinator.  It needs to 
> keep trying until the list is exhausted, and it shouldn't sleep between 
> attempts.
> 3. GMSLocator wasn't returning registrants for use in 
> findCoordinatorFromView().  This was causing it to choose itself as the 
> coordinator instead of using registrant sort order and choosing a different 
> registrant as the coordinator.
> 4. During concurrent startup GMSLocator didn't know when the decision was 
> made to become coordinator.  It is now notified of this decision and 
> processRequest() uses this flag to have it override anything in the 
> registrants set or in the recovered view.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
>  26b03276b0abbf6210a5602a8c551abe38edc261 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  c6bef571134c6444a297cc8fe0bb0b7eb95f41f4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java
>  c5fdf45411581a36feca220e14a0551f3197d368 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
>  edfaf625e6c652f46d9323c1116791f1c69fda59 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  93fa9dab4ec2c8e43fc41cfd3b8ad986f96cf00f 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  8abcc456e42ad00a558a93f87bd3ae74ce88d146 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java
>  c7b1a26b47cf2c913d9de30d6934ad5b3ac49840 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  390824eb11a2e72a21d951539c2e03ed8025be82 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 7ecca6146f6b7a542ae9864d7fabd48c9794ecac 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
>  df1d8d1101a5f9d04c402922955a283353aa3b7c 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
>  19cee066a488198471ebf4093045853e36d5ba78 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java
>  7f64

Re: Review Request 60106: GEODE-3052 Restarting 2 locators within 1s of each other causes potential locator split brain

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60106/#review178595
---



not sure you saw following commant, as I was having trouble with reviewboard.


geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 (Diff revision 2)
235 
if (v == null) {
we are setting -1 viewid for recorded view while recovery. Thus do we need 
"usingRecoveredView" flag here?

- Hitesh Khamesra


On June 21, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60106/
> ---
> 
> (Updated June 21, 2017, 10:24 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Brian Rowe.
> 
> 
> Bugs: GEODE-3052
> https://issues.apache.org/jira/browse/GEODE-3052
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There were four problems that new unit tests hit:
> 1. when recovering a view from disk we were treating it as a definitive 
> (live) view.  I've moved it to a new variable in GMSLocator and set its 
> viewId to -1.  At the same time I set the initial GMSJoinLeave 
> SearchState.viewId to -100 so it will be overridden by the one returned by 
> the locator.  These changes allow GmsJoinLeave to know that the potential 
> coordinator is from a recovered view.
> 2. when trying to join with a recovered view GMSJoinLeave.join() was giving 
> up after the second ID in the view and becoming the coordinator.  It needs to 
> keep trying until the list is exhausted, and it shouldn't sleep between 
> attempts.
> 3. GMSLocator wasn't returning registrants for use in 
> findCoordinatorFromView().  This was causing it to choose itself as the 
> coordinator instead of using registrant sort order and choosing a different 
> registrant as the coordinator.
> 4. During concurrent startup GMSLocator didn't know when the decision was 
> made to become coordinator.  It is now notified of this decision and 
> processRequest() uses this flag to have it override anything in the 
> registrants set or in the recovered view.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
>  26b03276b0abbf6210a5602a8c551abe38edc261 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  c6bef571134c6444a297cc8fe0bb0b7eb95f41f4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java
>  c5fdf45411581a36feca220e14a0551f3197d368 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
>  edfaf625e6c652f46d9323c1116791f1c69fda59 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  93fa9dab4ec2c8e43fc41cfd3b8ad986f96cf00f 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  8abcc456e42ad00a558a93f87bd3ae74ce88d146 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java
>  c7b1a26b47cf2c913d9de30d6934ad5b3ac49840 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  390824eb11a2e72a21d951539c2e03ed8025be82 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 7ecca6146f6b7a542ae9864d7fabd48c9794ecac 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
>  df1d8d1101a5f9d04c402922955a283353aa3b7c 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
>  19cee066a488198471ebf4093045853e36d5ba78 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java
>  7f64c670400464aa8e6a73405516bd6e891a006b 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java
>  b35270e2d97930cee68d8c54221a04c20dfb96de 
> 
> 
> Diff: https://reviews.apache.org/r/60106/diff/2/
> 
> 
> Testing
> ---
> 
> New unit tests, regression testing (under way), precheckin (under way)
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 60106: GEODE-3052 Restarting 2 locators within 1s of each other causes potential locator split brain

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60106/#review178617
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 21, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60106/
> ---
> 
> (Updated June 21, 2017, 10:24 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Brian Rowe.
> 
> 
> Bugs: GEODE-3052
> https://issues.apache.org/jira/browse/GEODE-3052
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There were four problems that new unit tests hit:
> 1. when recovering a view from disk we were treating it as a definitive 
> (live) view.  I've moved it to a new variable in GMSLocator and set its 
> viewId to -1.  At the same time I set the initial GMSJoinLeave 
> SearchState.viewId to -100 so it will be overridden by the one returned by 
> the locator.  These changes allow GmsJoinLeave to know that the potential 
> coordinator is from a recovered view.
> 2. when trying to join with a recovered view GMSJoinLeave.join() was giving 
> up after the second ID in the view and becoming the coordinator.  It needs to 
> keep trying until the list is exhausted, and it shouldn't sleep between 
> attempts.
> 3. GMSLocator wasn't returning registrants for use in 
> findCoordinatorFromView().  This was causing it to choose itself as the 
> coordinator instead of using registrant sort order and choosing a different 
> registrant as the coordinator.
> 4. During concurrent startup GMSLocator didn't know when the decision was 
> made to become coordinator.  It is now notified of this decision and 
> processRequest() uses this flag to have it override anything in the 
> registrants set or in the recovered view.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
>  26b03276b0abbf6210a5602a8c551abe38edc261 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  c6bef571134c6444a297cc8fe0bb0b7eb95f41f4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java
>  c5fdf45411581a36feca220e14a0551f3197d368 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
>  edfaf625e6c652f46d9323c1116791f1c69fda59 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  93fa9dab4ec2c8e43fc41cfd3b8ad986f96cf00f 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  8abcc456e42ad00a558a93f87bd3ae74ce88d146 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java
>  c7b1a26b47cf2c913d9de30d6934ad5b3ac49840 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  390824eb11a2e72a21d951539c2e03ed8025be82 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 7ecca6146f6b7a542ae9864d7fabd48c9794ecac 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
>  df1d8d1101a5f9d04c402922955a283353aa3b7c 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
>  19cee066a488198471ebf4093045853e36d5ba78 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java
>  7f64c670400464aa8e6a73405516bd6e891a006b 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java
>  b35270e2d97930cee68d8c54221a04c20dfb96de 
> 
> 
> Diff: https://reviews.apache.org/r/60106/diff/2/
> 
> 
> Testing
> ---
> 
> New unit tests, regression testing (under way), precheckin (under way)
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 60394: GEODE-3075 and GEODE-2995: merge of new protobuf protocol work.

[Hitesh Khamesra]

> On June 23, 2017, 6:38 p.m., Udo Kohlmeyer wrote:
> >

Will fix this latter on. But merging the code now.


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60394/#review178803
---


On June 23, 2017, 5:52 p.m., Galen O'Sullivan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60394/
> ---
> 
> (Updated June 23, 2017, 5:52 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This is a merge of https://reviews.apache.org/r/60157 and 
> https://reviews.apache.org/r/60217/ , adding an integration test and a couple 
> small changes. It's probably easiest to read in comparison to those diffs, or 
> in the git history at https://github.com/apache/geode/pull/597 .
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> 9a3241b05 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2a8818cef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e0b5ab8b6 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolMessageHandler.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/LegacyServerConnection.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  947b83652 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServiceLoadingFailureException.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java
>  794c61097 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
>  28c6c8a69 
>   geode-protobuf/build.gradle PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/exception/InvalidProtocolMessageException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationHandler.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/OperationsHandlerRegistry.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerAlreadyRegisteredException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerNotRegisteredException.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/EncodingTypeTranslator.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufSerializationService.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufUtilities.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/serializer/ProtobufProtocolSerializer.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/serializer/ProtocolSerializer.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationService.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationType.java
>  PRE-CREATION 
>   geode-protobuf/src/main/java/org/apache/geode/serialization/TypeCodec.java 
> PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/seria

Re: Review Request 60442: GEODE-3130: Refactoring AcceptorImpl communication mode switch

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60442/#review178916
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 26, 2017, 6:31 p.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60442/
> ---
> 
> (Updated June 26, 2017, 6:31 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3130
> https://issues.apache.org/jira/browse/GEODE-3130
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This review addresses Udo's last feedback for GEODE-2995.  This change also 
> cleaned up the imports on the AcceptorImplJUnitTest.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  50f7006fa 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplJUnitTest.java
>  7aa11b7ca 
> 
> 
> Diff: https://reviews.apache.org/r/60442/diff/1/
> 
> 
> Testing
> ---
> 
> Precheckin in progress.
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>

Re: Review Request 60446: Events do not get removed from the client queue for 1.0 clients

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60446/#review178936
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 26, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60446/
> ---
> 
> (Updated June 26, 2017, 10:24 p.m.)
> 
> 
> Review request for geode and Barry Oglesby.
> 
> 
> Bugs: GEODE-3072
> https://issues.apache.org/jira/browse/GEODE-3072
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> EventID and ThreadIdentifier hold the serialized form of portions of an 
> InternalDistributedMember identifier.  This serialized form changed in v1.0.0 
> and was causing .equals and .hashCode for these objects to return false when 
> they should have returned true owing to additional data being in the 
> serialized form of the identifier.
> 
> This change set modifies the equals and hashCode methods of the classes to 
> ensure that they return the correct results in the presence of this 
> additional ID data.
> 
> I created a dunit test to reproduce the problem but I think the behavior of 
> HARegionQueues isn't reliable enough to check in that test.  I'm afraid that 
> it would end up being a "flaky" test that fails periodically.  Instead, I'm 
> relying on a new junit test and the test that Barry already checked in.
> 
> Note: I've also included two other things in this change set.
> 
> First, LocalRegion.dumpBackingMap() is a test-hook that should log its 
> results at "info" level.  I used it in debugging this problem.
> 
> Second, I've added a new backward-compatibility version so now we have 100 
> and 110.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  a1b4a9d5684d0148bd9e72c00c674afa299b2b9d 
>   geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java 
> 4d2ddc11bb1bd36446dc30b2033a6bc1ed455c98 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ha/ThreadIdentifier.java
>  ec165a5af210966241fdc1cee81702231557cc8c 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
>  29b22be6cc866217f165b755f11e68e1343843fe 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
>  5fb8fa201b4c059d5d458a6af0915273f600f69f 
>   geode-old-versions/build.gradle d85eb0b7dea6d3fa6b19a0030e64433cb4cb7cb9 
> 
> 
> Diff: https://reviews.apache.org/r/60446/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/98d794d9-6e21-4a1b-8ee5-2394ac2baa6f__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/cc4dfcc9-dc0b-48da-b97b-337563b127fe__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/de1405e6-38f0-4972-adf7-e43e568ff5ad__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/8f143bb9-0225-4e3b-ace1-09614cf5efe8__diffall.txt
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

New Apache commons-validato.jar

[Hitesh Khamesra]

We are planning to include Apache commons-validator_1.6.jar in Geode. As part 
of GEODE-2804, we need to validate whether host string is configured as IP or 
not. Please let us know if there is any issue with it.

Thanks.
Hitesh

Re: Review Request 60451: GEODE-2996: adding Put handler

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60451/#review179015
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 27, 2017, 1:20 a.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60451/
> ---
> 
> (Updated June 27, 2017, 1:20 a.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-2996
> https://issues.apache.org/jira/browse/GEODE-2996
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This is a continuation of the review Alex submitted this morning with the 
> following changes:
> 
> Addresses review feedback for GEODE-2996, mainly refactoring 
> getOpertionHandler to handle failures like the putOperationHandler
> Adding put operations to the RoundTripCacheConnectionJUnitTest, which is the 
> integration test for the protobuf module
> Removing service loading for protobuf operations and instead have the 
> ProtobufStreamProcessor populate its OperationHandlerRegistry
> Remove exception throwing from OperationHandler.process calls and remove 
> TypeEncodingException
> Fixing ProtobufOpsProcessor to handle responses for types other than get
> 
> 
> Diffs
> -
> 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationHandler.java
>  7683e3bf3 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/OperationsHandlerRegistry.java
>  8e3a33149 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
>  d426149e4 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java
>  d7b5d4bd2 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufUtilities.java
>  d76366298 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
>  d9c14752f 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/PutRequestOperationHandler.java
>  PRE-CREATION 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/exception/TypeEncodingException.java
>  f3145a774 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/exception/UnsupportedEncodingTypeException.java
>  c577e768a 
>   
> geode-protobuf/src/main/java/org/apache/geode/serialization/registry/exception/CodecNotRegisteredForTypeException.java
>  5c923a520 
>   geode-protobuf/src/main/proto/region_API.proto 52291c451 
>   geode-protobuf/src/test/java/org/apache/geode/protocol/MessageUtil.java 
> f0b0b417b 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java
>  b9faca3c9 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessorJUnitTest.java
>  fc980aec9 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandlerJUnitTest.java
>  daa5870ed 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/PutRequestOperationHandlerJUnitTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60451/diff/1/
> 
> 
> Testing
> ---
> 
> Unit tests, whole module test, precheckin in progress.
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>

Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60312/
---

(Updated June 27, 2017, 8:04 p.m.)


Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
O'Sullivan.


Changes
---

Updated patch with latest develop


Repository: geode


Description
---

GEODE-2804 Cache InetAddress if configure host as ip string.

1. We keep configure host string in HostAddress class
2. We reuse InetsocketAddress if it is ipString.
3. Client has logic to retry thus we keep InetsocketAddress even if 
   it is not ip string.

GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.

GEODE-2940 Now we don't validate configure host string on start. As
there is possibility that host may not available.

1. Earlier wan config were failing because of that. Now we just keep
those configure host string. And try this later.

Added unit tests for it.


Diffs (updated)
-

  geode-assembly/build.gradle 39bb542 
  geode-assembly/src/test/resources/expected_jars.txt 6260167 
  geode-core/build.gradle 7f34b4a 
  
geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
 c1bfc93 
  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 53d401a 
  geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
3ded54a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 01c6157 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 a4b3a50 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
 da295ab 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 aff1938 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 bc3d708 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 6b54170 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
 5ab1bed 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
 1dc2fd1 
  
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 92cfd96 
  geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
d4fdbd0 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 0efba01 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 88832ba 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
 789d326 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 3cc3cfc 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 9f6c5fb 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java
 9d63050 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
 a31fa8d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
6a6e0c1 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorDiscovery.java
 f5a8fcf 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 d6d5d7c 
  
geode-wan/src/main/java/org/apache/geode/internal/cache/wan/AbstractRemoteGatewaySender.java
 dbc2cc6 
  
geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/WanAutoDiscoveryDUnitTest.java
 6d75064 
  gradle/dependency-versions.properties 6a730a4 


Diff: https://reviews.apache.org/r/60312/diff/2/

Changes: https://reviews.apache.org/r/60312/diff/1-2/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 60507: GEODE-3145: add geode-protobuf code to the geode jar

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60507/#review179135
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 28, 2017, 5:26 p.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60507/
> ---
> 
> (Updated June 28, 2017, 5:26 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3145
> https://issues.apache.org/jira/browse/GEODE-3145
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This change ensures that the protobuf handling code we've been working on is 
> actually available to geode users.
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle 39bb54215 
> 
> 
> Diff: https://reviews.apache.org/r/60507/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>

Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60312/
---

(Updated June 28, 2017, 6:48 p.m.)


Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
O'Sullivan.


Changes
---

Updated revied feedback


Repository: geode


Description
---

GEODE-2804 Cache InetAddress if configure host as ip string.

1. We keep configure host string in HostAddress class
2. We reuse InetsocketAddress if it is ipString.
3. Client has logic to retry thus we keep InetsocketAddress even if 
   it is not ip string.

GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.

GEODE-2940 Now we don't validate configure host string on start. As
there is possibility that host may not available.

1. Earlier wan config were failing because of that. Now we just keep
those configure host string. And try this later.

Added unit tests for it.


Diffs (updated)
-

  geode-assembly/build.gradle 39bb542 
  geode-assembly/src/test/resources/expected_jars.txt 6260167 
  geode-core/build.gradle 7f34b4a 
  
geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
 c1bfc93 
  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 53d401a 
  geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
3ded54a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 01c6157 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 a4b3a50 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
 da295ab 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 aff1938 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 bc3d708 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 6b54170 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
 5ab1bed 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
 1dc2fd1 
  
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 92cfd96 
  geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
d4fdbd0 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 0efba01 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 88832ba 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
 789d326 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 3cc3cfc 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 9f6c5fb 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java
 9d63050 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
 a31fa8d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
6a6e0c1 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorDiscovery.java
 f5a8fcf 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 d6d5d7c 
  
geode-wan/src/main/java/org/apache/geode/internal/cache/wan/AbstractRemoteGatewaySender.java
 dbc2cc6 
  
geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/WanAutoDiscoveryDUnitTest.java
 6d75064 
  gradle/dependency-versions.properties 6a730a4 


Diff: https://reviews.apache.org/r/60312/diff/3/

Changes: https://reviews.apache.org/r/60312/diff/2-3/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 60550: GEODE-3154: add geode-protobuf to expected_jars.txt

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60550/#review179309
---


Ship it!




Ship It!

- Hitesh Khamesra


On June 29, 2017, 9:39 p.m., Galen O'Sullivan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60550/
> ---
> 
> (Updated June 29, 2017, 9:39 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Barry Oglesby, Bruce Schuchardt, 
> Hitesh Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Currently BundledJarsJUnitTest is failing because we added geode-protobuf to 
> geode-assembly but not to the `expected_jars.txt` file, which tells us which 
> jars we expect. This one-liner adds it. This should keep builds from failing.
> 
> 
> Diffs
> -
> 
>   geode-assembly/src/test/resources/expected_jars.txt 62601677f 
> 
> 
> Diff: https://reviews.apache.org/r/60550/diff/1/
> 
> 
> Testing
> ---
> 
> Built on my machine, verified that the test passes.
> 
> 
> Thanks,
> 
> Galen O'Sullivan
> 
>

Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60312/
---

(Updated June 29, 2017, 10:27 p.m.)


Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
O'Sullivan.


Changes
---

Updated diff


Repository: geode


Description
---

GEODE-2804 Cache InetAddress if configure host as ip string.

1. We keep configure host string in HostAddress class
2. We reuse InetsocketAddress if it is ipString.
3. Client has logic to retry thus we keep InetsocketAddress even if 
   it is not ip string.

GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.

GEODE-2940 Now we don't validate configure host string on start. As
there is possibility that host may not available.

1. Earlier wan config were failing because of that. Now we just keep
those configure host string. And try this later.

Added unit tests for it.


Diffs
-

  geode-assembly/build.gradle 39bb542 
  geode-assembly/src/test/resources/expected_jars.txt 6260167 
  geode-core/build.gradle 7f34b4a 
  
geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
 c1bfc93 
  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 53d401a 
  geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
3ded54a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 01c6157 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 a4b3a50 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
 da295ab 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 aff1938 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 bc3d708 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 6b54170 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
 5ab1bed 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
 1dc2fd1 
  
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 92cfd96 
  geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
d4fdbd0 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 0efba01 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 88832ba 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
 789d326 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 3cc3cfc 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 9f6c5fb 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java
 9d63050 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
 a31fa8d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
6a6e0c1 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorDiscovery.java
 f5a8fcf 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 d6d5d7c 
  
geode-wan/src/main/java/org/apache/geode/internal/cache/wan/AbstractRemoteGatewaySender.java
 dbc2cc6 
  
geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/WanAutoDiscoveryDUnitTest.java
 6d75064 
  gradle/dependency-versions.properties 6a730a4 


Diff: https://reviews.apache.org/r/60312/diff/3/


Testing
---


File Attachments (updated)


Latest diff
  
https://reviews.apache.org/media/uploaded/files/2017/06/29/45369a6d-34d0-42dd-96bc-a1c009d00824__GEODE-2804v2.patch


Thanks,

Hitesh Khamesra

Review Request 60856: GEODE-3052 Need to reset isCoordinator flag in GMSLocator.

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60856/
---

Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
O'Sullivan, and Udo Kohlmeyer.


Repository: geode


Description
---

isCoordinator flag ensures that this process is becoming the
coordinator thus other process should join this process. But
when network parttion happens, we were not resetting this flag.

Now we reset isCoordinator flag when viewCreator thread shutdowns.

added unit test for it.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 2c56f5b 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 9591673 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
 d8c12e2 


Diff: https://reviews.apache.org/r/60856/diff/1/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 60856: GEODE-3052 Need to reset isCoordinator flag in GMSLocator.

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60856/
---

(Updated July 14, 2017, 5:06 p.m.)


Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
O'Sullivan, and Udo Kohlmeyer.


Changes
---

Changed log message to  debug


Repository: geode


Description
---

isCoordinator flag ensures that this process is becoming the
coordinator thus other process should join this process. But
when network parttion happens, we were not resetting this flag.

Now we reset isCoordinator flag when viewCreator thread shutdowns.

added unit test for it.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 2c56f5b 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 9591673 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
 d8c12e2 


Diff: https://reviews.apache.org/r/60856/diff/2/

Changes: https://reviews.apache.org/r/60856/diff/1-2/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 60856: GEODE-3052 Need to reset isCoordinator flag in GMSLocator.

[Hitesh Khamesra]

> On July 14, 2017, 12:18 a.m., Bruce Schuchardt wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
> > Lines 288 (patched)
> > <https://reviews.apache.org/r/60856/diff/1/?file=1776283#file1776283line288>
> >
> > remove debug logging or set to debug/trace level.  If you keep it you 
> > should use info("GMSLocator has coordinator flag {}", isCoordinator)

changed log message to debug.


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60856/#review180497
---


On July 14, 2017, 5:06 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60856/
> ---
> 
> (Updated July 14, 2017, 5:06 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> isCoordinator flag ensures that this process is becoming the
> coordinator thus other process should join this process. But
> when network parttion happens, we were not resetting this flag.
> 
> Now we reset isCoordinator flag when viewCreator thread shutdowns.
> 
> added unit test for it.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  2c56f5b 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  9591673 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
>  d8c12e2 
> 
> 
> Diff: https://reviews.apache.org/r/60856/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: [VOTE] Apache Geode release - v1.2.0 RC2

[Hitesh Khamesra]

+1
same as Dan!!

On Wednesday, July 12, 2017, 1:38:44 PM PDT, Dan Smith  
wrote:

+0

I ran geode-release-check
 against this
project. It all looks good except that the md5sum and sha256 sums tasks
don't pass because the .md5 and .sha256 files are in the wrong format. I
created GEODE-3196. I don't think that should hold up the release.

There's another issue with the example source though. It looks like trying
to build an example is prompting me for me gpg key so it can sign the
artifacts. Given that our README is telling users to run ../gradlew build
this is a bit annoying.

Awesome job making the examples build point at 1.2 but allowing me to
override the repo url so we can actually test them!

-Dan

On Wed, Jul 12, 2017 at 7:58 AM, Anthony Baker  wrote:

> This is the second release candidate for Apache Geode, version 1.2.0.
> Thanks to all the community members for their contributions to this
> release!  This release candidate fixes a number of backwards compatibility
> and rolling upgrade issues found in RC1.
>
> *** Please download, test and vote by Saturday, July 15, 0800 hrs
> US Pacific. ***
>
> It fixes the following issues:
>
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> projectId=12318420&version=12339257
>
> Note that we are voting upon the source tags:  rel/v1.2.0.RC2
>
> https://git-wip-us.apache.org/repos/asf?p=geode.git;a=commit;h=
> 964f2749065ce9c6898fd27983b43f1bd9fc77d0
>
> https://git-wip-us.apache.org/repos/asf?p=geode-examples.git;a=commit;h=
> 7f93d95ad06a6f2afee54312585f48435fff11e8
>
> Commit ID:
>  7f93d95ad06a6f2afee54312585f48435fff11e8 (geode)
>  7f93d95ad06a6f2afee54312585f48435fff11e8 (geode-examples)
>
> Source and binary files:
>  https://dist.apache.org/repos/dist/dev/geode/1.2.0.RC2
>
> Maven staging repo:
>  https://repository.apache.org/content/repositories/orgapachegeode-1020
>
> Geode's KEYS file containing PGP keys we use to sign the release:
>
> https://git-wip-us.apache.org/repos/asf?p=geode.git;a=blob_
> plain;f=KEYS;hb=HEAD
>
> pub  4096R/C72CFB64 2015-10-01
>      Fingerprint=948E 8234 14BE 693A 7F74  ABBE 19DB CAEE C72C FB64
>
> Anthony
>

Review Request 61411: GEODE-3286 Failing to cleanup connections from ConnectionTable receiver table (corrected "stopped" check in previous fix)

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61411/
---

Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
O'Sullivan, Udo Kohlmeyer, and Brian Rowe.


Repository: geode


Description
---

In previous fix, we were checking "if thread is stopped then add connection to 
receiver list". Instead, it should be if thread is stopped then we should not 
add connection to receiver list. As a fix, we add connection to reciver list if 
connection is not closed or thread is not stopped.

Added unit for it.


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java 
c3ad596 
  geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java 
69fb7a2 
  
geode-core/src/test/java/org/apache/geode/internal/tcp/ConnectionTableTest.java 
312c64d 


Diff: https://reviews.apache.org/r/61411/diff/1/


Testing
---


Thanks,

Hitesh Khamesra

Review Request 61420: GEODE-3307 CI failure: Uncaught exception in thread Thread[Geode Membership View Creator

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61420/
---

Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
O'Sullivan, Udo Kohlmeyer, and Brian Rowe.


Repository: geode


Description
---

Now we catch DistributedSystemDisconnectedException in ViewCreatoe thread and 
then mark shutdown true


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 40a4254 


Diff: https://reviews.apache.org/r/61420/diff/1/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 61411: GEODE-3286 Failing to cleanup connections from ConnectionTable receiver table (corrected "stopped" check in previous fix)

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61411/
---

(Updated Aug. 3, 2017, 11:01 p.m.)


Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
O'Sullivan, Udo Kohlmeyer, and Brian Rowe.


Changes
---

Updated diff based on review


Repository: geode


Description
---

In previous fix, we were checking "if thread is stopped then add connection to 
receiver list". Instead, it should be if thread is stopped then we should not 
add connection to receiver list. As a fix, we add connection to reciver list if 
connection is not closed or thread is not stopped.

Added unit for it.


Diffs (updated)
-

  geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java 
c3ad596 
  geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java 
69fb7a2 
  
geode-core/src/test/java/org/apache/geode/internal/tcp/ConnectionTableTest.java 
312c64d 


Diff: https://reviews.apache.org/r/61411/diff/2/

Changes: https://reviews.apache.org/r/61411/diff/1-2/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 61816: GEODE-3409 Protobuf Client Can't Connect Once Connection Limit Has Been Reached, Even After Connections Closed

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61816/#review183615
---


Ship it!




Ship It!

- Hitesh Khamesra


On Aug. 23, 2017, 3:47 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61816/
> ---
> 
> (Updated Aug. 23, 2017, 3:47 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Bugs: GEODE-3409
> https://issues.apache.org/jira/browse/GEODE-3409
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> ServerConnection cleanup was not decrementing the Acceptor's client 
> connection count when the protobuf communications mode was in effect.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  35cc33f3fa5547a01368a3ae3b6ad401b858610d 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  394d2614c0d04353c95454c8730e84688e4766fe 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java
>  12cc08b2f6cac77dcc2b79f1d3245f0960c3e8ba 
> 
> 
> Diff: https://reviews.apache.org/r/61816/diff/2/
> 
> 
> Testing
> ---
> 
> new integration test
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 61829: GEODE-3408 Flood of EOF warnings

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61829/#review183616
---




geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
Line 68 (original), 72 (patched)
<https://reviews.apache.org/r/61829/#comment259684>

Will this warning be printed every time when we close the socket?


- Hitesh Khamesra


On Aug. 22, 2017, 11:41 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61829/
> ---
> 
> (Updated Aug. 22, 2017, 11:41 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Bugs: GEODE-3408
> https://issues.apache.org/jira/browse/GEODE-3408
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Modified the message handler to catch EOFException and effect shutdown of the 
> connection & server thread.  I also fixed the handling of IOException.
> 
> The methods invoked are the same as for the old protocol's handling of 
> EOFException and IOException.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
>  cd1647a 
> 
> 
> Diff: https://reviews.apache.org/r/61829/diff/1/
> 
> 
> Testing
> ---
> 
> as this just affects logging behavior there are no new unit tests.  I 
> verified that the behavior is correct with the manual test used to reproduce 
> GEODE-3409 (my other review that is currently open).
> 
> precheckin is running now.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 61829: GEODE-3408 Flood of EOF warnings

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61829/#review183634
---


Ship it!




Ship It!

- Hitesh Khamesra


On Aug. 22, 2017, 11:41 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61829/
> ---
> 
> (Updated Aug. 22, 2017, 11:41 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Bugs: GEODE-3408
> https://issues.apache.org/jira/browse/GEODE-3408
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Modified the message handler to catch EOFException and effect shutdown of the 
> connection & server thread.  I also fixed the handling of IOException.
> 
> The methods invoked are the same as for the old protocol's handling of 
> EOFException and IOException.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
>  cd1647a 
> 
> 
> Diff: https://reviews.apache.org/r/61829/diff/1/
> 
> 
> Testing
> ---
> 
> as this just affects logging behavior there are no new unit tests.  I 
> verified that the behavior is correct with the manual test used to reproduce 
> GEODE-3409 (my other review that is currently open).
> 
> precheckin is running now.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 61950: GEODE-3519 servers are not locking on remove or invalidate ops initiated by clients

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61950/#review183950
---


Ship it!




Ship It!

- Hitesh Khamesra


On Aug. 28, 2017, 4:09 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61950/
> ---
> 
> (Updated Aug. 28, 2017, 4:09 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: geode-3519
> https://issues.apache.org/jira/browse/geode-3519
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> While investigating dlocktoken cleanup I discovered that a number of 
> operations coming from a client were not locking entries for Scope.GLOBAL 
> regions on servers.  Only put, putIfAbsent and variants of replace were 
> obtaining locks.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java
>  d9ed4edeff629b87d0fce2463fa9b22723b57d9a 
>   
> geode-core/src/test/java/org/apache/geode/cache30/ClientServerCCEDUnitTest.java
>  b4224e0dcba91ee75cf1abf185f6fc9fbe22390f 
> 
> 
> Diff: https://reviews.apache.org/r/61950/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin, new integration test
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 61978: GEODE-3059: LoadMonitor.connectionClosed incrementing statistics only for client-server connection

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61978/#review184391
---


Ship it!




Ship It!

- Hitesh Khamesra


On Aug. 30, 2017, 8:48 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61978/
> ---
> 
> (Updated Aug. 30, 2017, 8:48 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3059
> https://issues.apache.org/jira/browse/GEODE-3059
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The stat should only be incremented/decremented for certain kinds of 
> connections.  I've modified it to include protobuf connections.  All of the 
> constant byte identifiers in Acceptor.java have been moved to an enum in 
> CommunicationMode.java.  In that class I've added some "isa" checks to 
> replace the many big "if" checks for different kinds of connection modes.
> 
> A new connection modes will henceforth need to be added to 
> CommunicationMode.java where the appropriate "isa" methods can be updated to 
> include the new mode.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
>  dea8644bf6604a48b38e0f8a9fcfa48deb4b56c8 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
>  078844f7adc907761c8b0b1b5525874c8338144e 
>   
> geode-core/src/main/java/org/apache/geode/cache/server/internal/LoadMonitor.java
>  1c571a924f5517f7ba1a04e216c3641f96f9ddc4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
>  83f87eebe3f7cbea628107078e1bafac478a0228 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> e12a409bc556ab74718830ff8036edd6216ef53b 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/ClientHandShake.java
>  f7a39f3c86f261d76133b2bebb864f53be027f1a 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2e33af897a852a2f397cf3c1d7f3b20ae3b1d69f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientNotifier.java
>  e2612fc45ccf59ee64654380b771835415780f9d 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e877852a42009c0cadeaa0e69516bdbcd84e6bd4 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListener.java
>  104d88abc99d3b329834f53d558b4684b6f9c226 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListenerAdapter.java
>  7476b4fba5def66c52b1fa26fb3f0f2e4c63fd17 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  690fd83971d9ac9f8ecedf9b89acb7ade887f38e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  6f56e85382ded1668bc51f6c0f2cc4990a493750 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
>  00e8b8880a5f336bb82c578f29acaf256187bb5c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerHandShakeProcessor.java
>  47e6f3d0bd9cab249e8f92398b116c6111b0e60e 
>   
> geode-core/src/test/java/org/apache/geode/cache/server/internal/LoadMonitorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnectionTest.java
>  ea0001867e5a22025f4dbca4ead172ff25f2af4d 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
>  cffa05fa49de58187eaab5bb10c7f2a6ed3bf0f5 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java
>  2aa89954659594428ae3f6ed6d7146261d518203 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  fbd582a1fa3c0e7b0bce033924f38d45ba2c2a9e 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/AuthenticationIntegrationTest.java
>  122b8e3ba7ecab407fdedf6be35153a1def728ff 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripLocatorConnectionJUnitTest.java
>  7ee307b4068247245fc02ab1ae7266c7e2e385c3 
> 
> 
> Diff: https://reviews.apache.org/r/61978/diff/3/
> 
> 
> Testing
> ---
> 
> precheckin is running now
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 62088: GEODE-3249 Validate internal client/server messages

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62088/#review184565
---


Ship it!




Ship It!

- Hitesh Khamesra


On Sept. 5, 2017, 5:57 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62088/
> ---
> 
> (Updated Sept. 5, 2017, 5:57 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3249
> https://issues.apache.org/jira/browse/GEODE-3249
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This change leaves the security hole in place but allows you to plug it by 
> setting the system property
> 
> geode.disallow-internal-messages-without-credentials=true
> 
> Clients must be upgraded to the release containing this change if you set 
> this system property to true and client/server authentication is enabled.  
> Otherwise client messages to register PDX types or Instantiators will be 
> rejected by the servers.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  b243d8ebb8f7fb698a4637c7a787ee2d7216f1f7 
> 
> 
> Diff: https://reviews.apache.org/r/62088/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 62088: GEODE-3249 Validate internal client/server messages

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62088/#review184986
---


Ship it!




Ship It!

- Hitesh Khamesra


On Sept. 7, 2017, 5:43 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62088/
> ---
> 
> (Updated Sept. 7, 2017, 5:43 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3249
> https://issues.apache.org/jira/browse/GEODE-3249
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This change leaves the security hole in place but allows you to plug it by 
> setting the system property
> 
> geode.disallow-internal-messages-without-credentials=true
> 
> Clients must be upgraded to the release containing this change if you set 
> this system property to true and client/server authentication is enabled.  
> Otherwise client messages to register PDX types or Instantiators will be 
> rejected by the servers.
> 
> New tests have been added to perform backward-compatibility testing with the 
> old security implementation and the internal message command classes have 
> been modified to perform validation of credentials if the system property is 
> set to true.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  b243d8ebb8f7fb698a4637c7a787ee2d7216f1f7 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxEnum.java
>  5a4a07b81b18d33e465bd3aa46ad4232b976b608 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxType.java
>  041e12fbd04e81f1f69520c53ef9c2f7481132fd 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetFunctionAttribute.java
>  76cc4a59bff691c4760083861362825d70ba326e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXEnumById.java
>  5e59640e5067ec8ac5fc50807ec276e1bdc025dd 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForEnum.java
>  b0ebaf23f27e91278c7afe3648954ad6113206a8 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForType.java
>  f2172ef4d8fa9f83929d8f5b2aa0c5377d7cf57e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXTypeById.java
>  e46445bc96d735a66aa09330a1790b951591251e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxEnums70.java
>  3fe9750f8577a70e4cda9e76da83070f6e6606b1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxTypes70.java
>  e64683fb620985d698357912bb1d1b52e8b24681 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterDataSerializers.java
>  eef5195eae3bedb414aa2e2fca748b31e0b27908 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInstantiators.java
>  a402cb360f05f99442833e6098c736d2ac18d69a 
>   
> geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationDUnitTest.java
>  ca7b2b6b7a2c8d8215eda828901a05dcabdf3625 
>   
> geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationPart2DUnitTest.java
>  f8ebe056e21228f1d9e32e1dd271f6a4bfb4af71 
>   
> geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationTestCase.java
>  0ecd72f4ee321f7f8aa5e998fa176551e45f025c 
>   
> geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationDUnitTest.java
>  09aedbec86f95ab9affa1f76b387a5a03c0098ec 
>   
> geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationTestCase.java
>  a4fd365ffaa51447d56c2bcb481311082ddcbc31 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtils.java 
> e69f36de1efbd0061ad8621db45fe3a64686968e 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/MonitorCQ.java
>  f5e31df988f5955d2fbeef5269a7729ec97c9d03 
>   
> geode-cq/src/test/java/org/apache/geode/security/ClientAuthorizationTwoDUnitTest.java
>  f5f686c0595c7500c4275292edb2e8f87593c67e 
> 
> 
> Diff: https://reviews.apache.org/r/62088/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: New client/server protocol - seeking feedback

[Hitesh Khamesra]

 +1
On Monday, October 2, 2017, 11:14:55 AM PDT, Jacob Barrett 
 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  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 
> wrote:
>
> > Replies inline.
> >
> > On Mon, Sep 25, 2017 at 12:13 PM, Udo Kohlmeyer 
> > wrote:
> >
> > > Replies inline
> > > On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith  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: [VOTE] C++ standardize on return values only

[Hitesh Khamesra]

Tuple option.

Sent from Yahoo Mail on Android 
 
  On Tue, Oct 3, 2017 at 4:27 PM, Jacob Barrett wrote:   
Voting on the conversation around C++ return values vs. out parameters.
This vote is to adopt the standard of return values over the use of out
parameters. On functions that must return more than one value to use the
C++11 std::tuple type for future compatibility with C++17.

For example:

std::tuple foo::getAAndB() {...}

And call it with:

int a;
std::string b;
std::tie(a, b) = foo.getAAndB();

Alternatively the tuple can be called like:

auto r = foo.getAAndB();
auto a = std::get<0>(r);
auto b = std::get<1>(r);

In C++17:

auto [a, b] = foo.getAAndB();



Rather than:

int foo::getAAndB(std::string& b) {...}

Called like

std::string b;
auto a = foo.getAAndB(b);

-Jake
  

Re: [DISCUSS] Release branch for 1.1.0

[Hitesh Khamesra]

 GEODE-2413 : Implementing a peer-to-peer protocol for mutual auth require some 
more investigation. Apart from that, we need to support backward compatibility 
as well.  Considering the complexity of the issue, would it make sense to move 
that out of 1.1.0 release?

Thoughts?

 

  From: Hitesh Khamesra 
 To: "dev@geode.apache.org"  
 Sent: Thursday, February 2, 2017 10:33 AM
 Subject: Re: [DISCUSS] Release branch for 1.1.0
   
We want to fix one more issue in this release
 GEODE-2413 peer-to-peer authentication: Peer need to re-authenticate 
coordinator while accepting view message 

GEODE-2386    Unable to launch dunit VMs in nightly builds
>>IMO, we can move forward with 1.1.0 since these failures seem to be related 
>>to the Jenkins environment.  Thoughts?
+1

Thanks.Hitesh



      From: Anthony Baker 
 To: dev@geode.apache.org 
 Sent: Thursday, February 2, 2017 9:16 AM
 Subject: Re: [DISCUSS] Release branch for 1.1.0
  
Looks like the tests are failing even with your changes.  They pass for me when 
run locally.  Perhaps it’s time to look into dockerizing the jenkins 
environment.

IMO, we can move forward with 1.1.0 since these failures seem to be related to 
the Jenkins environment.  Thoughts?

Anthony

> On Feb 1, 2017, at 2:34 PM, Dan Smith  wrote:
> 
> I checked in what I hope is a workaround for GEODE-2386. We'll see what
> happens when the nightly build runs. It doesn't seem to reproduce in other
> environments.
> 
> -Dan
> 
> On Wed, Feb 1, 2017 at 1:34 PM, Anthony Baker  wrote:
> 
>> If it doesn’t need to be fixed in 1.1.0, please unset the ‘Fix Version’ in
>> JIRA.
>> 
>> Thanks,
>> Anthony
>> 
>> 
>>> On Feb 1, 2017, at 9:53 AM, Kevin Duling  wrote:
>>> 
>>> GEODE-2247 GFSH over HTTP succeeds without authentication
>>> 
>>> The title for this is a little misleading.  Yes, it succeeds, but with an
>>> 'anonymous' and unprivileged user.  That could be a valid use-case.  For
>>> example, dev rest does not require a login to execute a ping.
>>> 
>>> I hope to have it resolved today, but in my opinion, it's not critical
>>> enough to hold up a release.
>>> 
>>> On Wed, Feb 1, 2017 at 9:46 AM, Anthony Baker  wrote:
>>> 
>>>> While we’re finalizing the last fixes, let’s crowd-source the release
>>>> notes.  These will be linked from the releases page on the website and
>>>> included in the ANNOUNCE email.  You can edit the release notes here:
>>>> 
>>>> https://cwiki.apache.org/confluence/display/GEODE/Release+Notes
>>>> 
>>>> Thanks,
>>>> Anthony
>>>> 
>>>>> On Jan 31, 2017, at 3:40 PM, Hitesh Khamesra
>> 
>>>> wrote:
>>>>> 
>>>>> Update: We are waiting for two more fixes.
>>>>> GEODE-2386 Unable to launch dunit VMs in nightly builds
>>>>> GEODE-2247 GFSH over HTTP succeeds without authentication
>>>>> Thanks,Hitesh
>>>>> 
>>>>> 
>>>>>    From: Hitesh Khamesra 
>>>>> To: "dev@geode.apache.org" 
>>>>> Sent: Monday, January 30, 2017 2:55 PM
>>>>> Subject: Re: [DISCUSS] Release branch for 1.1.0
>>>>> 
>>>>> Thanks Bruce. There is one more ticket GEODE-2386, which Kirk is
>> looking
>>>> into it.
>>>>> -Hitesh
>>>>> 
>>>>> 
>>>>>    From: Bruce Schuchardt 
>>>>> To: dev@geode.apache.org
>>>>> Sent: Monday, January 30, 2017 2:21 PM
>>>>> Subject: Re: [DISCUSS] Release branch for 1.1.0
>>>>> 
>>>>> I'm done merging these two changes to release/1.1.0
>>>>> 
>>>>> Le 1/30/2017 à 10:58 AM, Hitesh Khamesra a écrit :
>>>>>> Sure. Lets include GEODE-2368 (Need to fix log message in
>>>> DirectChannel) this as well.
>>>>>> Thanks.Hitesh
>>>>>> 
>>>>>> 
>>>>>>      From: Bruce Schuchardt 
>>>>>> To: dev@geode.apache.org
>>>>>> Sent: Monday, January 30, 2017 10:37 AM
>>>>>> Subject: Re: [DISCUSS] Release branch for 1.1.0
>>>>>> 
>>>>>> I'd like to merge into 1.1.0 the change to the Host test class that I
>>>>>> checked into develop today.  It's breaking things for some people, so
>> it
>>>>>> would be nice to have in the 1.1.0 branch.
>>>>>> 
>>>>>> Le 1/27/2017 à 11:00 AM, Hitesh Khamesra a écrit :
>>>>>>> I have created the release branch "release/1.1.0". Please look at
>> this
>>>> and raise if there is any issue.
>>>>>>> If there is no concern then we will start voting next week.
>>>>>>> Thanks.Hitesh
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>> 
>>>> 
>> 
>> 


  

   

Re: [DISCUSS] Release branch for 1.1.0

[Hitesh Khamesra]

Thanks, I have moved this out from 1.1.0.
-Hitesh


  From: Anilkumar Gingade 
 To: dev@geode.apache.org 
 Sent: Friday, February 3, 2017 2:44 PM
 Subject: Re: [DISCUSS] Release branch for 1.1.0
   
Agree, no need to push changes in hurry...

-Anil.


On Fri, Feb 3, 2017 at 2:33 PM, Dan Smith  wrote:

> +1 for pushing out GEODE-2413. I think what Anthony said makes sense.
>
> -Dan
>
> On Fri, Feb 3, 2017 at 2:32 PM, Anthony Baker  wrote:
>
> >
> > > On Feb 3, 2017, at 2:03 PM, Hitesh Khamesra
> 
> > wrote:
> > >
> > >  GEODE-2413 : Implementing a peer-to-peer protocol for mutual auth
> > require some more investigation. Apart from that, we need to support
> > backward compatibility as well.  Considering the complexity of the issue,
> > would it make sense to move that out of 1.1.0 release?
> > >
> > > Thoughts?
> > >
> >
> > Sounds like it has the potential to destabilize the release branch.  I
> > think we should move it out of the release so we can take the time to
> > properly develop tests and discuss the best solution.
> >
> > Anthony
> >
> >
>


   

[VOTE] RC1: Apache Geode release - v1.1.0

[Hitesh Khamesra]

All,

This is the first release candidate of the first release for Apache Geode, 
version 1.1.0.
Thanks to all the community members.

It fixes the following issues:
   
https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12318420&version=12338352


*** Please download, test and vote by Wednesday, February 08, 0800 hrs US 
Pacific.

Note that we are voting upon the source (tag):
   rel/v1.1.0.RC1
   
https://git-wip-us.apache.org/repos/asf?p=geode.git;a=tag;h=refs/tags/rel/v1.1.0.RC1

Commit ID: 90760d334f912cef49ad32f275aed9cd9f1f2688
   
Source and binary files:
   https://dist.apache.org/repos/dist/dev/geode/1.1.0.RC1/

Maven staging repo:
   https://repository.apache.org/content/repositories/orgapachegeode-1015/

Geode's KEYS file containing PGP keys we use to sign the release:
   https://github.com/apache/geode/blob/release/1.1.0/KEYS

pub   4096R/AC6AB8ED 2017-01-18
  Key fingerprint = 048F B40F AD7D 9C15 54AD  D447 2CA9 90A2 AC6A B8ED

Thanks,
Dan & Hitesh (on behalf of the Geode team)

Re: Fwd: Build failed in Jenkins: Geode-nightly #738

[Hitesh Khamesra]

Will disable it  testSSLEnabledLocatorDiesWhenConnectingToNonSSLLocator test.
-Hitesh


  From: Kirk Lund 
 To: geode  
 Sent: Monday, February 6, 2017 9:05 AM
 Subject: Fwd: Build failed in Jenkins: Geode-nightly #738
   
Can someone please do one of the following (I don't have build job
permissions)?

a) disable FlakyTest in the nightly build

or

b) add FlakyTest results to the test results

Also, I think we need to either fix
testSSLEnabledLocatorDiesWhenConnectingToNonSSLLocator or disable it. It's
failing frequently in the nightly build.

:geode-core:flakyTest

org.apache.geode.distributed.LocatorDUnitTest >
testSSLEnabledLocatorDiesWhenConnectingToNonSSLLocator FAILED
    org.apache.geode.test.dunit.RMIException: While invoking
org.apache.geode.distributed.LocatorDUnitTest$$Lambda$26/981119691.run in
VM 2 running on Host asf920.gq1.ygridcore.net with 4 VMs
        at org.apache.geode.test.dunit.VM.invoke(VM.java:377)
        at org.apache.geode.test.dunit.VM.invoke(VM.java:347)
        at org.apache.geode.test.dunit.VM.invoke(VM.java:292)
        at
org.apache.geode.distributed.LocatorDUnitTest.testSSLEnabledLocatorDiesWhenConnectingToNonSSLLocator(LocatorDUnitTest.java:535)

        Caused by:
        com.jayway.awaitility.core.ConditionTimeoutException: Condition
with alias 'locator2 dies' didn't complete within 30 seconds because
condition with org.apache.geode.distributed.LocatorDUnitTest was not
fulfilled.
            at
com.jayway.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:122)
            at
com.jayway.awaitility.core.CallableCondition.await(CallableCondition.java:79)
            at
com.jayway.awaitility.core.CallableCondition.await(CallableCondition.java:27)
            at
com.jayway.awaitility.core.ConditionFactory.until(ConditionFactory.java:764)
            at
com.jayway.awaitility.core.ConditionFactory.until(ConditionFactory.java:741)
            at
org.apache.geode.distributed.LocatorDUnitTest.lambda$testSSLEnabledLocatorDiesWhenConnectingToNonSSLLocator$bb17a952$1(LocatorDUnitTest.java:536)

org.apache.geode.distributed.LocatorUDPSecurityDUnitTest >
testSSLEnabledLocatorDiesWhenConnectingToNonSSLLocator FAILED
    org.apache.geode.test.dunit.RMIException: While invoking
org.apache.geode.distributed.LocatorDUnitTest$$Lambda$17/407365607.run in
VM 2 running on Host asf920.gq1.ygridcore.net with 4 VMs

        Caused by:
        com.jayway.awaitility.core.ConditionTimeoutException: Condition
with alias 'locator2 dies' didn't complete within 30 seconds because
condition with org.apache.geode.distributed.LocatorDUnitTest was not
fulfilled.

268 tests completed, 2 failed, 6 skipped
:geode-core:flakyTest FAILED
-- Forwarded message --
From: Apache Jenkins Server 
Date: Sun, Feb 5, 2017 at 8:10 AM
Subject: Build failed in Jenkins: Geode-nightly #738
To: dev@geode.apache.org, dbar...@pivotal.io, gz...@pivotal.io,
aba...@pivotal.io, e...@pivotal.io, huyn...@gmail.com, aging...@pivotal.io,
kdul...@pivotal.io, upthewatersp...@apache.org, bogle...@pivotal.io,
n...@pivotal.io, bschucha...@pivotal.io, hkhame...@pivotal.io,
kl...@pivotal.io, ukohlme...@pivotal.io, hitesh...@yahoo.com


See 

--
[...truncated 549 lines...]
            at com.jayway.awaitility.core.ConditionFactory.until(
ConditionFactory.java:741)
            at org.apache.geode.distributed.LocatorDUnitTest.lambda$
testSSLEnabledLocatorDiesWhenConnectingToNonSSLLocator$
bb17a952$1(LocatorDUnitTest.java:536)

org.apache.geode.distributed.LocatorUDPSecurityDUnitTest >
testSSLEnabledLocatorDiesWhenConnectingToNonSSLLocator FAILED
    org.apache.geode.test.dunit.RMIException: While invoking
org.apache.geode.distributed.LocatorDUnitTest$$Lambda$17/407365607.run in
VM 2 running on Host asf920.gq1.ygridcore.net with 4 VMs

        Caused by:
        com.jayway.awaitility.core.ConditionTimeoutException: Condition
with alias 'locator2 dies' didn't complete within 30 seconds because
condition with org.apache.geode.distributed.LocatorDUnitTest was not
fulfilled.

268 tests completed, 2 failed, 6 skipped
:geode-core:flakyTest FAILED
:geode-core:integrationTest
:geode-cq:assemble
:geode-cq:compileTestJavaNote: Some input files use or override a
deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-cq:processTestResources
:geode-cq:testClasses
:geode-cq:checkMissedTests
:geode-cq:spotlessJavaCheck
:geode-cq:spotlessCheck
:geode-cq:test
:geode-cq:check
:geode-cq:build
:geode-cq:distributedTest
:geode-cq:flakyTest
:geode-cq:integrationTest
:geode-json:assemble
:geode-json:compileTestJava UP-TO-DATE
:geode-json:processTestResources UP-TO-DATE
:geode-json:testClasses UP-TO-DATE
:geode-json:checkMissedTests UP-TO-DATE
:geode-json:spotlessJavaCheck
:geode-json:spotlessCheck
:geode-json:test UP-TO-DA

Re: [VOTE] RC1: Apache Geode release - v1.1.0

[Hitesh Khamesra]

GEODE-2430    Remove binary files from test resources (Kirk, Please merge in 
1.1)GEODE-2432 Don't create and upload empty maven artifacts for 
geode-benchmarks, geode-old-versions(Dan, please merge it)

Thanks.HItesh

  From: Kirk Lund 
 To: geode  
 Sent: Monday, February 6, 2017 3:44 PM
 Subject: Re: [VOTE] RC1: Apache Geode release - v1.1.0
   
These .zip and .jar files are now removed on develop. These two commits
should be merged if we want this fix on the release branch:

commit e769796c5611f4fad1a21869ddea29853ed1958e
Author: Jared Stewart 
Date:  Mon Feb 6 14:54:49 2017 -0800

    GEODE-2430: Remove jar and zip files from test resources

    This closes #393

commit 50aebcc859da9c2456ef142ff7ec4c1620c11900
Author: Jared Stewart 
Date:  Fri Feb 3 12:53:38 2017 -0800

    GEODE-2430: Refactor ZipUtils


On Mon, Feb 6, 2017 at 8:05 AM, Anthony Baker  wrote:

> -1 (binding)
>
> `gradle rat` shows these binary files are included in the src distribution:
>
>  + /Users/abaker/working/apache-geode-src-1.1.0/geode-core/
> src/test/resources/org/apache/geode/management/internal/
> configuration/cluster.jar
>
>  + /Users/abaker/working/apache-geode-src-1.1.0/geode-core/
> src/test/resources/org/apache/geode/management/internal/
> configuration/cluster_config.zip
>
>  + /Users/abaker/working/apache-geode-src-1.1.0/geode-core/
> src/test/resources/org/apache/geode/management/internal/
> configuration/cluster_config_security.zip
>
>  + /Users/abaker/working/apache-geode-src-1.1.0/geode-core/
> src/test/resources/org/apache/geode/management/internal/
> configuration/group1.jar
>
>  + /Users/abaker/working/apache-geode-src-1.1.0/geode-core/
> src/test/resources/org/apache/geode/management/internal/
> configuration/group2.jar
>
>
> If tests require binary files, those files should be generated from source
> during the build.
>
> Anthony
>
> > On Feb 3, 2017, at 4:11 PM, Hitesh Khamesra 
> wrote:
> >
> > All,
> >
> > This is the first release candidate of the first release for Apache
> Geode, version 1.1.0.
> > Thanks to all the community members.
> >
> > It fixes the following issues:
> >    https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> projectId=12318420&version=12338352
> >
> >
> > *** Please download, test and vote by Wednesday, February 08, 0800 hrs
> US Pacific.
> >
> > Note that we are voting upon the source (tag):
> >    rel/v1.1.0.RC1
> >    https://git-wip-us.apache.org/repos/asf?p=geode.git;a=tag;h=
> refs/tags/rel/v1.1.0.RC1
> >
> > Commit ID: 90760d334f912cef49ad32f275aed9cd9f1f2688
> >
> > Source and binary files:
> >    https://dist.apache.org/repos/dist/dev/geode/1.1.0.RC1/
> >
> > Maven staging repo:
> >    https://repository.apache.org/content/repositories/
> orgapachegeode-1015/
> >
> > Geode's KEYS file containing PGP keys we use to sign the release:
> >    https://github.com/apache/geode/blob/release/1.1.0/KEYS
> >
> > pub  4096R/AC6AB8ED 2017-01-18
> >      Key fingerprint = 048F B40F AD7D 9C15 54AD  D447 2CA9 90A2 AC6A
> B8ED
> >
> > Thanks,
> > Dan & Hitesh (on behalf of the Geode team)
>
>


   

Re: Nightly build broken

[Hitesh Khamesra]

Thanks Dan. Lets merge this fix into 1.1 release branch.
-Hitesh


  From: Dan Smith 
 To: dev@geode.apache.org 
 Sent: Tuesday, February 7, 2017 1:12 PM
 Subject: Re: Nightly build broken
   
Actually, now that I think about it GEODE-2434 was probably the cause of
these backwards compatibility test failures. The build was generating the
geodeOldVersionClasspaths file during the configuration phase. What that
means is if you run './gradlew clean precheckin', the file gets generated
*before* the clean, which will then delete it. Just doing this without the
fix for GEODE-2434 will also fail with that error:

./gradlew clean geode-core:distributedTest --tests
'*ClientServerMiscBCDUnitTest*'

We should merge the fix for GEODE-2434 to the release branch as well.

-Dan

On Tue, Feb 7, 2017 at 12:59 PM, Kirk Lund  wrote:

> The BC tests are failing with this RuntimeException (it ends up being an
> initializationError):
>
> java.lang.RuntimeException: No older versions of Geode were found to test
> against
> at
> org.apache.geode.internal.cache.tier.sockets.ClientServerMiscBCDUnitTest.
> data(ClientServerMiscBCDUnitTest.java:37)
>
>
> On Tue, Feb 7, 2017 at 12:55 PM, Udo Kohlmeyer 
> wrote:
>
> > Looking at some of the failures, it seems it is complaining about not
> > finding diskstores to delete.
> >
> >
> >
> > On 2/7/17 12:53, Kirk Lund wrote:
> >
> >> Last night's nightly build is pretty horrible. 121 dunit test failures.
> >> Cluster config accounts for 13 of them and we already have a fix for
> those
> >> 13 only.
> >>
> >> Anyone else want to look into the other failures? Looks like some of the
> >> other failures are diskstore related.
> >>
> >> https://builds.apache.org/job/Geode-nightly/ws/geode-core/bu
> >> ild/reports/distributedTest/index.html
> >>
> >> Failures:
> >>
> >> DistTXDebugDUnitTest. testTXDestroy_invalidate
> >> DistTXDebugDUnitTest. testTXPR
> >> DistTXDebugDUnitTest. testTXPR2
> >> DistTXDebugDUnitTest. testTXPRRR2_create
> >> DistTXDebugDUnitTest. testTXPRRR2_putall
> >> DistTXDebugDUnitTest. testTXPR_RR
> >> DistTXDebugDUnitTest. testTXPR_putall
> >> DistTXDebugDUnitTest. testTXPR_removeAll
> >> DistTXDebugDUnitTest. testTXRR2
> >> DistTXDebugDUnitTest. testTXRR2_dataNodeAsCoordinator
> >> DistTXDebugDUnitTest. testTXRR_removeAll
> >> DistTXDebugDUnitTest. testTXRR_removeAll_dataNodeAsCoordinator
> >> DistTXOrderDUnitTest. testBug43353
> >> DistTXOrderDUnitTest. testFarSideIndexOnDestroy
> >> DistTXOrderDUnitTest. testFarSideIndexOnInvalidate
> >> DistTXOrderDUnitTest. testFarSideIndexOnPut
> >> DistTXPersistentDebugDUnitTest. testBasicDistributedTX
> >> DistTXPersistentDebugDUnitTest. testTXDestroy_invalidate
> >> DistTXPersistentDebugDUnitTest. testTXPR
> >> DistTXPersistentDebugDUnitTest. testTXPR2
> >> DistTXPersistentDebugDUnitTest. testTXPRRR2_create
> >> DistTXPersistentDebugDUnitTest. testTXPRRR2_putall
> >> DistTXPersistentDebugDUnitTest. testTXPR_RR
> >> DistTXPersistentDebugDUnitTest. testTXPR_putall
> >> DistTXPersistentDebugDUnitTest. testTXPR_removeAll
> >> DistTXPersistentDebugDUnitTest. testTXRR2
> >> DistTXPersistentDebugDUnitTest. testTXRR2_dataNodeAsCoordinator
> >> DistTXPersistentDebugDUnitTest. testTXRR_removeAll
> >> DistTXPersistentDebugDUnitTest. testTXRR_removeAll_
> dataNodeAsCoordinator
> >> DistTXRestrictionsDUnitTest. testPersistentRestriction
> >> DistTXWithDeltaDUnitTest. testClientServerDelta
> >> DistTXWithDeltaDUnitTest. testExceptionThrown
> >> DistTXWithDeltaDUnitTest. testTxWithCloning
> >> DistributedTransactionDUnitTest. testBasicDistributedTX
> >> DistributedTransactionDUnitTest. testCommitAndRollback
> >> DistributedTransactionDUnitTest. testCommitConflicts_PR
> >> DistributedTransactionDUnitTest. testCommitConflicts_PR_after_l
> >> ocks_acquired
> >> DistributedTransactionDUnitTest. testCommitConflicts_RR
> >> DistributedTransactionDUnitTest. testCommitNoConflicts_PR
> >> DistributedTransactionDUnitTest. testCommitNoConflicts_RR
> >> DistributedTransactionDUnitTest. testCommitOnPartitionedAndRepl
> >> icatedRegions
> >> DistributedTransactionDUnitTest. testGetIsolated
> >> DistributedTransactionDUnitTest. testMultipleOpsOnSameKeyInTx
> >> DistributedTransactionDUnitTest. testNonColocatedPutByPartitioning
> >> DistributedTransactionDUnitTest. testPutAllWithTransactions
> >> DistributedTransactionDUnitTest. testRegionAndEntryVersionsPR
> >> DistributedTransactionDUnitTest. testRegionAndEntryVersionsRR
> >> DistributedTransactionDUnitTest. testRemoveAllWithTransactions
> >> DistributedTransactionDUnitTest. testTransactionalKeyBasedDestroys_PR
> >> DistributedTransactionDUnitTest. testTransactionalKeyBasedDestroys_RR
> >> DistributedTransactionDUnitTest. testTransactionalKeyBasedUpdates
> >> DistributedTransactionDUnitTest. testTransactionalPutOnPartitio
> nedRegion
> >> DistributedTransactionDUnitTest. testTransactionalPutOnReplicatedRegion
> >> DistributedTransactionDUnitTest. testTransactionalUpdates
> >> DistributedTransactionDUnitTest. t

[VOTE] RC2: Apache Geode release - v1.1.0

[Hitesh Khamesra]

All,

This is the second release candidate of the first release for Apache Geode, 
version 1.1.0.
Thanks to all the community members.

It fixes the following issues:
   
https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12318420&version=12338352


*** Please download, test and vote by Tuesday, February 14, 0800 hrs US Pacific.

Note that we are voting upon the source (tag):
   rel/v1.1.0.RC2
   
https://git-wip-us.apache.org/repos/asf?p=geode.git;a=tag;h=refs/tags/rel/v1.1.0.RC2

Commit ID:     2286fd064a52173eab8fdcfadfb89a01e81ef728
  
Source and binary files:
   https://dist.apache.org/repos/dist/dev/geode/1.1.0.RC2/

Maven staging repo:
   https://repository.apache.org/content/repositories/orgapachegeode-1016/

Geode's KEYS file containing PGP keys we use to sign the release:
   https://github.com/apache/geode/blob/release/1.1.0/KEYS

pub   4096R/AC6AB8ED 2017-01-18
  Key fingerprint = 048F B40F AD7D 9C15 54AD  D447 2CA9 90A2 AC6A B8ED

Thanks,
Hitesh

Re: Review Request 56520: Update dependency versions

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56520/#review165033
---


Ship it!




Ship It!

- Hitesh Khamesra


On Feb. 10, 2017, 12:07 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56520/
> ---
> 
> (Updated Feb. 10, 2017, 12:07 a.m.)
> 
> 
> Review request for geode, Anthony Baker, anilkumar gingade, Bruce Schuchardt, 
> Darrel Schneider, Hitesh Khamesra, Jacob Barrett, Jens Deppe, Jinmei Liao, 
> Jared Stewart, Kevin Duling, Udo Kohlmeyer, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Update several dependencies so that geode develop uses the latest version.
> 
> These changes fully pass precheckin. Please let me know which dependencies 
> should NOT be updated for any reason and I'll retest with a list of 
> dependencies that it's ok to update and file appropriate Jira ticket(s).
> 
> If there you see dependencies changed in the diff that require me to run any 
> tests not in precheckin, please let me know either in review or direct 
> message.
> 
> 
> Diffs
> -
> 
>   gradle/dependency-versions.properties a0b291e 
> 
> Diff: https://reviews.apache.org/r/56520/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 56556: GEODE-2454: use DistributedMember.getId() for memberId assertions

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56556/#review165168
---


Ship it!




Ship It!

- Hitesh Khamesra


On Feb. 10, 2017, 7:30 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56556/
> ---
> 
> (Updated Feb. 10, 2017, 7:30 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, Jinmei Liao, 
> Jared Stewart, Kevin Duling, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-2454
> https://issues.apache.org/jira/browse/GEODE-2454
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2454: use DistributedMember.getId() for memberId assertions
> 
> This test is failing in the Geode Nightly Build because the assertion is 
> incorrect. It asserts that the MembershipEvent.getMemberId() is equal to 
> DistributedMember.toString(). It should instead assert that 
> MembershipEvent.getMemberId() is equal to DistributedMember.getId(). There 
> are multiple places in the code where it was using toString() instead of 
> getId().
> 
> I also deleted old comments from the code.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/UniversalMembershipListenerAdapterDUnitTest.java
>  e0d3d8b 
> 
> Diff: https://reviews.apache.org/r/56556/diff/
> 
> 
> Testing
> ---
> 
> UniversalMembershipListenerAdapterDUnitTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 56564: GEODE-2449: Moved Redis out of Geode-core into its own module

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56564/#review165181
---




geode-core/src/main/java/org/apache/geode/redis/GeodeRedisService.java (line 22)
<https://reviews.apache.org/r/56564/#comment236990>

Make this some generic interface which can be used by any other. Next is 
mamcache ..


- Hitesh Khamesra


On Feb. 10, 2017, 9:39 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56564/
> ---
> 
> (Updated Feb. 10, 2017, 9:39 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, Hitesh 
> Khamesra, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Moved Geode-Redis out of core. No other changes other than code move and some 
> test clean up.
> Added GeodeRedisService Interface to be used for the ServiceLoader code in 
> GemFireCacheImpl
> 
> 
> Diffs
> -
> 
>   geode-core/build.gradle 8eba6d4e8 
>   
> geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
>  63f650510 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
>  fa6d13f7c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  6e374ecb7 
>   geode-core/src/main/java/org/apache/geode/internal/hll/Bits.java 595fb57ac 
>   
> geode-core/src/main/java/org/apache/geode/internal/hll/CardinalityMergeException.java
>  59ab0950e 
>   geode-core/src/main/java/org/apache/geode/internal/hll/HyperLogLog.java 
> 4bdf81c77 
>   geode-core/src/main/java/org/apache/geode/internal/hll/HyperLogLogPlus.java 
> fc4b6e554 
>   geode-core/src/main/java/org/apache/geode/internal/hll/IBuilder.java 
> 10189c8bc 
>   geode-core/src/main/java/org/apache/geode/internal/hll/ICardinality.java 
> 125b62183 
>   geode-core/src/main/java/org/apache/geode/internal/hll/MurmurHash.java 
> be19e29ae 
>   geode-core/src/main/java/org/apache/geode/internal/hll/RegisterSet.java 
> cad691b25 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/FixedPartitionAttributesInfo.java
>  eb0435a37 
>   geode-core/src/main/java/org/apache/geode/redis/GeodeRedisServer.java 
> 4c97c98bf 
>   geode-core/src/main/java/org/apache/geode/redis/GeodeRedisService.java 
> PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/ByteArrayWrapper.java
>  4a0ef5989 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/ByteToCommandDecoder.java
>  124bf7512 
>   geode-core/src/main/java/org/apache/geode/redis/internal/Command.java  
>   geode-core/src/main/java/org/apache/geode/redis/internal/DoubleWrapper.java 
> 60cd130da 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/ExecutionHandlerContext.java
>  e2b49bedc 
>   geode-core/src/main/java/org/apache/geode/redis/internal/Executor.java  
>   geode-core/src/main/java/org/apache/geode/redis/internal/Extendable.java  
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/RedisCommandParserException.java
>   
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
>   
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/RedisConstants.java 
> 3c39c01c5 
>   geode-core/src/main/java/org/apache/geode/redis/internal/RedisDataType.java 
> 63a15dff9 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/RedisDataTypeMismatchException.java
>   
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/RegionCreationException.java
>   
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/RegionProvider.java 
> 5994d7d8c 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/executor/AbstractExecutor.java
>  c9d47ab9b 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/executor/AbstractScanExecutor.java
>  0eb6dcad3 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/executor/AuthExecutor.java
>  9d318a450 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/executor/DBSizeExecutor.java
>   
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/executor/DelExecutor.java
>  e0db6518c 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/executor/EchoExecutor.java
>  407e65354 
>   
> geode-core/src/main/java/org/apache/geode/redis/internal/executor/ExistsExecutor.java
>  96611dc06 
&

Re: [VOTE] RC2: Apache Geode release - v1.1.0

[Hitesh Khamesra]

e-site/website/content/releases/index.html:  compile 
'org.apache.geode:geode-core:1.0.0-incubating'
geode-site/website/content/releases/index.html:    
<version>1.0.0-incubating</version>
geode-site/website/content/releases/index.html:  If you need access to 
older releases they can be found in the https://archive.apache.org/dist/incubator/geode/";>
geode-spark-connector/project/Dependencies.scala:    val geode = 
"org.apache.geode" % "geode-core" % "1.0.0-incubating" 
excludeAll(ExclusionRule(organization = "org.jboss.netty"


  From: Galen M O'Sullivan 
 To: dev@geode.apache.org 
Cc: Hitesh Khamesra 
 Sent: Friday, February 10, 2017 1:59 PM
 Subject: Re: [VOTE] RC2: Apache Geode release - v1.1.0
   
Good catch, Ken. It looks like the Dockerfile and Docker scripts refer to
'incubat*', and a few places in the docs do as well.

On Fri, Feb 10, 2017 at 1:35 PM, Kenneth Howe  wrote:

> +0
> I’m not going to give an outright down-vote, but BUILDING.md needs to be
> updated to refer to the correct versions.
>
> The build steps show the expected response to
>  gfsh version
> to be "v1.0.0-incubating”
>
> It’s not great to show the wrong version, but since this is the first
> release since graduation showing an incorrect “incubating” version tag is
> particularly disturbing. I’d hate to confuse any new users kicking the
> tires on geode now that it’s an Apache TLP.
>
> Ken
>
> > On Feb 9, 2017, at 12:55 PM, Hitesh Khamesra 
> wrote:
> >
> > All,
> >
> > This is the second release candidate of the first release for Apache
> Geode, version 1.1.0.
> > Thanks to all the community members.
> >
> > It fixes the following issues:
> >    https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> projectId=12318420&version=12338352
> >
> >
> > *** Please download, test and vote by Tuesday, February 14, 0800 hrs US
> Pacific.
> >
> > Note that we are voting upon the source (tag):
> >    rel/v1.1.0.RC2
> >    https://git-wip-us.apache.org/repos/asf?p=geode.git;a=tag;h=
> refs/tags/rel/v1.1.0.RC2
> >
> > Commit ID:    2286fd064a52173eab8fdcfadfb89a01e81ef728
> >
> > Source and binary files:
> >    https://dist.apache.org/repos/dist/dev/geode/1.1.0.RC2/
> >
> > Maven staging repo:
> >    https://repository.apache.org/content/repositories/
> orgapachegeode-1016/
> >
> > Geode's KEYS file containing PGP keys we use to sign the release:
> >    https://github.com/apache/geode/blob/release/1.1.0/KEYS
> >
> > pub  4096R/AC6AB8ED 2017-01-18
> >      Key fingerprint = 048F B40F AD7D 9C15 54AD  D447 2CA9 90A2 AC6A
> B8ED
> >
> > Thanks,
> > Hitesh
>
>

   

Re: Review Request 56626: GEODE-2479 Remove docs reference to gemstone.com package

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56626/#review165417
---




geode-docs/developing/data_serialization/auto_serialization.html.md.erb 
<https://reviews.apache.org/r/56626/#comment237267>

I thought this info is good to keep; Do we have similar info somewhere else?


- Hitesh Khamesra


On Feb. 14, 2017, 12:22 a.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56626/
> ---
> 
> (Updated Feb. 14, 2017, 12:22 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Dave Barnes, Hitesh Khamesra, 
> Joey McAllister, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2479 Remove docs reference to gemstone.com package
> 
> 
> Diffs
> -
> 
>   geode-docs/developing/data_serialization/auto_serialization.html.md.erb 
> cdb63a8fb5f5c5bcf7330bc2d611c3e126618b51 
> 
> Diff: https://reviews.apache.org/r/56626/diff/
> 
> 
> Testing
> ---
> 
> Gradle rat check passes.  I did not build Geode.  I did build the manual, and 
> no errors occur for a manual build.
> 
> Devs with serialization experience should double check that the note changed 
> is now correct with the package name update.
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

Re: Review Request 56626: GEODE-2479 Remove docs reference to gemstone.com package

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56626/#review165423
---


Ship it!




Ship It!

- Hitesh Khamesra


On Feb. 14, 2017, 12:22 a.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56626/
> ---
> 
> (Updated Feb. 14, 2017, 12:22 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Dave Barnes, Hitesh Khamesra, 
> Joey McAllister, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2479 Remove docs reference to gemstone.com package
> 
> 
> Diffs
> -
> 
>   geode-docs/developing/data_serialization/auto_serialization.html.md.erb 
> cdb63a8fb5f5c5bcf7330bc2d611c3e126618b51 
> 
> Diff: https://reviews.apache.org/r/56626/diff/
> 
> 
> Testing
> ---
> 
> Gradle rat check passes.  I did not build Geode.  I did build the manual, and 
> no errors occur for a manual build.
> 
> Devs with serialization experience should double check that the note changed 
> is now correct with the package name update.
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

[RESULT][VOTE] RC2: Apache Geode 1.1.0 release

[Hitesh Khamesra]

The vote passes with 6 +1 and 2 +0 votes:

Jinmei Liao  +1
Udo Kohlmeyer    +1
William Markito  +1
Dan Smith    +1
Xiaojian Zhou    +1 
Anthony Baker    +1

Kenneth Howe +0
Karen Miller +0

Mail thread: 
http://mail-archives.apache.org/mod_mbox/incubator-geode-dev/201702.mbox/%3C1656177403.1540917.1486673726802%40mail.yahoo.com%3E

Thanks.Dan & Hitesh

Merge conflict from release/1.1.0 branch to develop

[Hitesh Khamesra]

I was merging release/1.1.0 branch to develop to see if something we need to 
pull from there. Generally we don't expect anything from it but it is showing 
some conflicts. Can you(Bruce, Jenmei, Jared) please look following files.

-bash-4.2$ git merge --no-ff release/1.1.0Auto-merging gradle.properties
Auto-merging 
geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
CONFLICT (content): Merge conflict in 
geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
Auto-merging 
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
CONFLICT (content): Merge conflict in 
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
Auto-merging 
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
CONFLICT (content): Merge conflict in 
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
Auto-merging 
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
CONFLICT (content): Merge conflict in 
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
Auto-merging 
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java
CONFLICT (content): Merge conflict in 
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java
Auto-merging 
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
CONFLICT (content): Merge conflict in 
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
Auto-merging 
geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradeDUnitTest.java
CONFLICT (content): Merge conflict in 
geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradeDUnitTest.java
Auto-merging 
geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTest.java
CONFLICT (content): Merge conflict in 
geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTest.java
Auto-merging 
geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java
Auto-merging 
geode-core/src/main/java/org/apache/geode/distributed/internal/direct/DirectChannel.java
CONFLICT (content): Merge conflict in 
geode-core/src/main/java/org/apache/geode/distributed/internal/direct/DirectChannel.java
Automatic merge failed; fix conflicts and then commit the result.
-bash-4.2$

GeodeRedisAdapter improvments/feedback

[Hitesh Khamesra]

Current GeodeRedisAdapter implementation is based on 
https://cwiki.apache.org/confluence/display/GEODE/Geode+Redis+Adapter+Proposal.
We are looking for some feedback on Redis commands and their mapping to geode 
region.

1. Redis Type String
  a. Usage Set k1 v1
  b. Current implementation creates "STRING_REGION" geode-partition-region 
upfront
  c. This k1/v1 are geode-region key/value
  d. Any feedback?
 
2. List Type
  a. usage "rpush mylist A"
  b. Current implementation maps each list to geode-partition-region(i.e. 
mylist is geode-partition-region); with the ability to get item from head/tail
  c. Feedback/vote
  -- List type operation at region-entry level;
  -- region-key = "mylist"
  -- region-value = Arraylist (will support all redis list ops)
  d. Feedback/vote: both behavior is desirable
  

3. Hashes
  a. this represents field-value or java bean object
  b. usage "hmset user1000 username antirez birthyear 1977 verified 1"
  c. Current implementation maps each hashes to geode-partition-region(i.e. 
user1000 is geode-partition-region)
  d. Feedback/vote
    -- Should we map hashes to region-entry
    -- region-key = user1000
    -- region-value = map
    -- This will provide java bean sort to behaviour with 10s of field-value
    -- Personally I would prefer this..
  e. Feedback/vote: both behaviour is desirable

4. Sets
  a. This represents unique keys in set
  b. usage "sadd myset 1 2 3" 
  c. Current implementation maps each sadd to geode-partition-region(i.e. myset 
is geode-partition-region)
  d. Feedback/vote
    -- Should we map set to region-entry
    -- region-key = myset
    -- region-value = Hashset
  e. Feedback/vote: both behaviour is desirable

5. SortedSets
  a. This represents unique keys in set with score (usecase Query top-10)
  b. usage "zadd hackers 1940 "Alan Kay"" 
  c. Current implementation maps each zadd to geode-partition-region(i.e. 
hackers is geode-partition-region)
  d. Feedback/vote
    -- Should we map set to region-entry
    -- region-key = hackers
    -- region-value = Sorted Hashset
  e. Feedback/vote: both behaviour is desirable

6. HyperLogLogs
  a. A HyperLogLog is a probabilistic data structure used in order to count 
unique things (technically this is referred to estimating the cardinality of a 
set).
  b. usage "pfadd hll a b c d"
  c. Current implementation creates "HLL_REGION" geode-partition-region upfront
  d. hll becomes region-key and value is HLL object
  e. any feedback?
  
7. Default config for geode-region (vote) 
   a. partition region
   b. 1 redundant copy
   c. Persistence
   d. Eviction
   e. Expiration
   f. ?
   
8. It seems; redis knows type(list, hashes, string ,set ..) of each key. Thus 
for each operation we need to make sure type of key. In current implementation 
we have different region for each redis type. Thus we have another 
region(metaTypeRegion) which keeps type for each key. This makes any operation 
in geode slow as it needs to verify that type. For instance, creating new key 
need to make sure its already there or not. Whether we should allow type change 
or not.
  a. Feedback/vote 
 -- type change of key
 -- Can we allow two key with same name but two differnt type (as it will 
endup in two different geode-region)
    String type "key1" in string region
    HLL type "key1" in HLL region
  b. any other feedback
  
9. Transactions:
  a. we will not support transaction in redisAdapter as geode transaction are 
limited to single node.
  b. feedback?
  
10. Redis COMMAND (https://redis.io/commands/command)
  a. should we implement this "COMMAND" ?
  
11. Any other redis command we should consider?
 

Thanks.Hitesh

Re: Propose a new implementation for collections in Geode transaction (GEODE-2392)

[Hitesh Khamesra]

+1 for supporting repeatable read semantics consistently. throw 
Unsupportedexception if some apis are not supported inside tx. 
-Hitesh.  From: Anthony Baker 
 To: dev@geode.apache.org 
 Sent: Monday, February 13, 2017 8:08 AM
 Subject: Re: Propose a new implementation for collections in Geode transaction 
(GEODE-2392)
   
Questions:

1) Do you know why we don’t set detectReadConflicts [1] true by default?  I 
would guess that most users would not expect dirty reads in the context of a 
repeatable read isolation level.
2) In the case of non-local reads on a Partitioned Region, shouldn’t we throw a 
TransactionDataNotColocatedException?
3) Some cache operations throw an UnsupportedOperationInTransactionException.  
Is there a reason we don’t for containsValue()?


Anthony

[1] 
https://geode.apache.org/docs/guide/developing/transactions/transaction_semantics.html

> On Feb 10, 2017, at 12:23 PM, Anilkumar Gingade  wrote:
> 
> As Eric mentioned, this will address the current inconsistencies in the
> product and will provide consistent behavior with replicated and
> partitioned region.
> 
> Also, if we look into typical database transaction applications, the
> transaction queries doesn't touch/read all the records in the table, mostly
> or always they are targeted queries (with where clause) with small result
> set. In Geode case, without support for any filtering option with
> collection api's, it may have to bring in all the region entries into the
> Tx state, which application may not be looking for...The better solution to
> support this is by making Geode queries transactional; which can provide
> repeatable read semantic on sub-set data.
> 
> I agree, its hard for application to distinguish what apis are part of Tx
> and what is not...Option to address this could be, proper documentation or
> not allowing collection apis inside the Tx. In needed, application has to
> suspend the Tx, to call them.
> 
> -Anil.
> 
> 
> 
> 
> 
> 
> 
> 
> On Fri, Feb 10, 2017 at 10:57 AM, Eric Shu  wrote:
> 
>> Hi Dan,
>> 
>> Currently query results do not reflect transaction state at all. The new
>> proposal won't change that.
>> 
>> On Fri, Feb 10, 2017 at 10:47 AM, Eric Shu  wrote:
>> 
>>> Please note even in our current transaction implementation, repeatable
>>> read is not supported on collection operation in transaction with
>>> Partitioned Region. Currently, only the local entry set (resides in the
>>> local node) of a PR is copied into the txState, while data on the remote
>>> nodes are not. Another call of the collection operation of the same
>>> transaction could have different results for the data on the remote
>> nodes.
>>> 
>>> In addition, containValue() call currently does not support repeatable
>>> read either. It does not copy all data into txState (replicated or
>>> partitioned regions.)
>>> 
>>> Currently we only support limited repeatable read for collection
>>> operations in transaction (at least for PR).
>>> 
>>> On Fri, Feb 10, 2017 at 8:36 AM, Anthony Baker 
>> wrote:
>>> 
 I tend to agree with Mike.  Removing collection reads from the txn state
 changes the programming model and makes it harder to write a Geode
 application.  We’re leaking internal details into the public behavior.
 
 Is there another way to provide this optimization to the user?  Perhaps
 something like:
 
    CacheTransactionManager txMgr = cache.getCacheTransactionManager();
    txMgr.begin(READ_COMMITTED);  // just thinking out loud here
 
 Or, we could set a hard cap on the size of txState.  Would that satisfy
 the original goal to ensure that users apply Geode txns correctly?
 
 Anthony
 
> On Feb 10, 2017, at 12:15 AM, Michael Stolz 
>> wrote:
> 
> -1
> 
> I would recommend against breaking repeatable read semantics for
 specific
> cases. If we're going to do something about the memory pressure,
>> great,
 but
> not at the cost of functionality guarantees.
> 
> --
> Mike Stolz
> Principal Engineer - Gemfire Product Manager
> Mobile: 631-835-4771
> 
> On Feb 9, 2017 10:29 PM, "Eric Shu"  wrote:
> 
> All,
> 
> In current Geode transaction implementation, there will be memory
 pressure
> if collection is used in a transaction. (GEODE-2392
> https://issues.apache.org/jira/browse/GEODE-2392).
> 
> Geode transactions provide repeatable read semantics. In order to
 support
> this repeatable read isolation, all read operations copy the current
 value
> in txState.
> 
> The proposed new implementation is to have collection operation in a
> transaction not copying all values into its txState. Instead, it will
>> go
> over to region directly but reflecting the new state of the entries
 changed
> by the previous operations of this transaction.
> 
> Please note that the proposed implementation will not support
>> repeatable
> 

Re: GeodeRedisAdapter improvments/feedback

[Hitesh Khamesra]

Jason/Dan: Sorry to hear about that. But both of you have asked the right 
question.
it depends on your use-case(item 2,3,4,5) . For example "hashes" can be use to 
define key-value pair or java bean. In this case  probably it is better to keep 
that hash at region-entry level.  But if you want to know top 10 tweets which 
are trending then probably you want use partition-region for "sorted-set". 


  From: Jason Huynh 
 To: dev@geode.apache.org; "u...@geode.apache.org" ; 
Hitesh Khamesra  
 Sent: Tuesday, February 14, 2017 3:15 PM
 Subject: Re: GeodeRedisAdapter improvments/feedback
   
Hi Hitesh,

Not sure about everyone else, but I had a hard time reading this,  however
I think I figured out what you were describing... the only part I still am
unsure about is  Feedback/vote: both behaviour is desirable.  Do you mean
you want feedback and voting on whether both behaviors are desired?  As in
old implementation and new implementation?

2,3,4)  The new implementation would mean all the data for a specific data
structure is contained in a single bucket.  So the individual data
structures are not quite scalable.  How would you allow scaling of a single
data structure?

On Tue, Feb 14, 2017 at 3:05 PM Real Wes  wrote:

> In what format do you want the feedback Hitesh?  For now I’ll just comment:
>
> 1. Redis Type String
> No comments except that a future Geode value-add would be to extend the
> Jedis client so that the K/V’s are not compressed. In this way OQL and CQ
> will work.  The tradeoff of this is that the data cannot be read by a
> native redis client but for Geode users it’s great. Call the new client
> Geodis.
>
> 2. List/ Hash/ Set/ SortedSet
> Creating a separate region for each creates a constraint that the keys are
> limited to the characters for region names, which are A-z/0-9/ - and _.
> Everything else is out. Redis users might start asking questions why their
> list named ++^^/## throws an error. Your suggestion to make it a key rather
> than a region solves this. Furthermore, creating a new region every time a
> new Redis collection is created is going to be slow. I’m not sure why a
> region was created but I’m sure it made sense to the developer at the time.
>
> 7. Default Config
> Can’t we configure a gfsh option to default to the region types we want?
> Customer A will want PARTITION but Customer B will want
> PARTITION_REDUNDANT_EXPIRATION_PERSISTENT.  I wonder if we can consider a
> geode> create region —redisType=PARTITION_REDUNDANT_EXPIRATION_PERSISTENT
> that makes _all_ Redis regions of that type?
>
>
>
> On Feb 14, 2017, at 5:36 PM, Hitesh Khamesra  hitesh...@yahoo.com>> wrote:
>
> Current GeodeRedisAdapter implementation is based on
> https://cwiki.apache.org/confluence/display/GEODE/Geode+Redis+Adapter+Proposal
> .
> We are looking for some feedback on Redis commands and their mapping to
> geode region.
>
> 1. Redis Type String
>  a. Usage Set k1 v1
>  b. Current implementation creates "STRING_REGION" geode-partition-region
> upfront
>  c. This k1/v1 are geode-region key/value
>  d. Any feedback?
>
> 2. List Type
>  a. usage "rpush mylist A"
>  b. Current implementation maps each list to geode-partition-region(i.e.
> mylist is geode-partition-region); with the ability to get item from
> head/tail
>  c. Feedback/vote
>      -- List type operation at region-entry level;
>      -- region-key = "mylist"
>      -- region-value = Arraylist (will support all redis list ops)
>  d. Feedback/vote: both behavior is desirable
>
>
> 3. Hashes
>  a. this represents field-value or java bean object
>  b. usage "hmset user1000 username antirez birthyear 1977 verified 1"
>  c. Current implementation maps each hashes to
> geode-partition-region(i.e. user1000 is geode-partition-region)
>  d. Feedback/vote
>    -- Should we map hashes to region-entry
>    -- region-key = user1000
>    -- region-value = map
>    -- This will provide java bean sort to behaviour with 10s of
> field-value
>    -- Personally I would prefer this..
>  e. Feedback/vote: both behaviour is desirable
>
> 4. Sets
>  a. This represents unique keys in set
>  b. usage "sadd myset 1 2 3"
>  c. Current implementation maps each sadd to geode-partition-region(i.e.
> myset is geode-partition-region)
>  d. Feedback/vote
>    -- Should we map set to region-entry
>    -- region-key = myset
>    -- region-value = Hashset
>  e. Feedback/vote: both behaviour is desirable
>
> 5. SortedSets
>  a. This represents unique keys in set with score (usecase Query top-10)
>  b. usage "zadd hackers 1940 "Alan Kay""
>  c. Current implementation maps each zadd to geode-partition-region(i.e.
&g

Re: GeodeRedisAdapter improvments/feedback

[Hitesh Khamesra]

How's going on. What are you working these days.

Sent from Yahoo Mail on Android 
 
  On Tue, Feb 14, 2017 at 4:16 PM, William Markito 
Oliveira wrote:   Definitely not by asking in the 
middle of on-going  discussion threads. 

Instructions: http://bfy.tw/A5ub :)

Sent from my iPhone

> On Feb 14, 2017, at 6:38 PM, Michael Vos  wrote:
> 
> How do I unsubscribe from this email list?
> 
> Thank you, 
> 
> Michael Vos
> Strategic Partnerships
> 310-804-7223 | m...@pivotal.io
> 
> 
> 
> 
> 
>> On Feb 14, 2017, at 3:37 PM, Dan Smith  wrote:
>> 
>> I also had a hard time reading this. It sounds like the tl;dr is that your 
>> are proposing storing each redis collection in a single region entry, rather 
>> than a a partition region? I guess the question is whether users will have a 
>> few very large collections that need to be partitioned, or lots and lots of 
>> really tiny collections. I'm not quite sure what the more common use case is.
>> 
>> -Dan
>> 
>>> On Tue, Feb 14, 2017 at 3:22 PM, Swapnil Bawaskar  
>>> wrote:
>>> The Redis adapter was designed so that we can scale all the Redis data 
>>> structures horizontally. If you bring the data structures to region entry 
>>> level, there is no reason for anyone to use our implementation over Redis.
>>> 
>>>> On Tue, Feb 14, 2017 at 3:15 PM Jason Huynh  wrote:
>>>> Hi Hitesh,
>>>> 
>>>> Not sure about everyone else, but I had a hard time reading this,  however 
>>>> I think I figured out what you were describing... the only part I still am 
>>>> unsure about is  Feedback/vote: both behaviour is desirable.  Do you mean 
>>>> you want feedback and voting on whether both behaviors are desired?  As in 
>>>> old implementation and new implementation?
>>>> 
>>>> 2,3,4)  The new implementation would mean all the data for a specific data 
>>>> structure is contained in a single bucket.  So the individual data 
>>>> structures are not quite scalable.  How would you allow scaling of a 
>>>> single data structure?
>>>> 
>>>> On Tue, Feb 14, 2017 at 3:05 PM Real Wes  wrote:
>>>> In what format do you want the feedback Hitesh?  For now I’ll just comment:
>>>> 
>>>> 1. Redis Type String
>>>> No comments except that a future Geode value-add would be to extend the 
>>>> Jedis client so that the K/V’s are not compressed. In this way OQL and CQ 
>>>> will work.  The tradeoff of this is that the data cannot be read by a 
>>>> native redis client but for Geode users it’s great. Call the new client 
>>>> Geodis.
>>>> 
>>>> 2. List/ Hash/ Set/ SortedSet
>>>> Creating a separate region for each creates a constraint that the keys are 
>>>> limited to the characters for region names, which are A-z/0-9/ - and _.  
>>>> Everything else is out. Redis users might start asking questions why their 
>>>> list named ++^^/## throws an error. Your suggestion to make it a key 
>>>> rather than a region solves this. Furthermore, creating a new region every 
>>>> time a new Redis collection is created is going to be slow. I’m not sure 
>>>> why a region was created but I’m sure it made sense to the developer at 
>>>> the time.
>>>> 
>>>> 7. Default Config
>>>> Can’t we configure a gfsh option to default to the region types we want?  
>>>> Customer A will want PARTITION but Customer B will want 
>>>> PARTITION_REDUNDANT_EXPIRATION_PERSISTENT.  I wonder if we can consider a 
>>>> geode> create region —redisType=PARTITION_REDUNDANT_EXPIRATION_PERSISTENT 
>>>> that makes _all_ Redis regions of that type?
>>>> 
>>>> 
>>>> 
>>>> On Feb 14, 2017, at 5:36 PM, Hitesh Khamesra 
>>>> mailto:hitesh...@yahoo.com>> wrote:
>>>> 
>>>> Current GeodeRedisAdapter implementation is based on 
>>>> https://cwiki.apache.org/confluence/display/GEODE/Geode+Redis+Adapter+Proposal.
>>>> We are looking for some feedback on Redis commands and their mapping to 
>>>> geode region.
>>>> 
>>>> 1. Redis Type String
>>>>  a. Usage Set k1 v1
>>>>  b. Current implementation creates "STRING_REGION" geode-partition-region 
>>>>upfront
>>>>  c. This k1/v1 are geode-region key/value
>>>>  d. Any feedback?
>>>> 
>>>> 2. List Type
>>>>  a. usage &quo

Re: GeodeRedisAdapter improvments/feedback

[Hitesh Khamesra]

>>>The Redis adapter was designed so that we can scale all the Redis data
structures horizontally. If you bring the data structures to region entry
level, there is no reason for anyone to use our implementation over Redis.
hmm, here we need to understand when we need to create partition-region for 
Redis data types(item 2,3,4,5)
Creating partition-region for each use case may not be feasible. See a couple 
of use-cases mentioned earlier in the thread. 



 
On Tue, Feb 14, 2017 at 3:15 PM Jason Huynh  wrote:

> Hi Hitesh,
>
> Not sure about everyone else, but I had a hard time reading this,  however
> I think I figured out what you were describing... the only part I still am
> unsure about is  Feedback/vote: both behaviour is desirable.  Do you mean
> you want feedback and voting on whether both behaviors are desired?  As in
> old implementation and new implementation?
>
> 2,3,4)  The new implementation would mean all the data for a specific data
> structure is contained in a single bucket.  So the individual data
> structures are not quite scalable.  How would you allow scaling of a single
> data structure?
>
> On Tue, Feb 14, 2017 at 3:05 PM Real Wes  wrote:
>
> In what format do you want the feedback Hitesh?  For now I’ll just comment:
>
> 1. Redis Type String
> No comments except that a future Geode value-add would be to extend the
> Jedis client so that the K/V’s are not compressed. In this way OQL and CQ
> will work.  The tradeoff of this is that the data cannot be read by a
> native redis client but for Geode users it’s great. Call the new client
> Geodis.
>
> 2. List/ Hash/ Set/ SortedSet
> Creating a separate region for each creates a constraint that the keys are
> limited to the characters for region names, which are A-z/0-9/ - and _.
> Everything else is out. Redis users might start asking questions why their
> list named ++^^/## throws an error. Your suggestion to make it a key rather
> than a region solves this. Furthermore, creating a new region every time a
> new Redis collection is created is going to be slow. I’m not sure why a
> region was created but I’m sure it made sense to the developer at the time.
>
> 7. Default Config
> Can’t we configure a gfsh option to default to the region types we want?
> Customer A will want PARTITION but Customer B will want
> PARTITION_REDUNDANT_EXPIRATION_PERSISTENT.  I wonder if we can consider a
> geode> create region —redisType=PARTITION_REDUNDANT_EXPIRATION_PERSISTENT
> that makes _all_ Redis regions of that type?
>
>
>
> On Feb 14, 2017, at 5:36 PM, Hitesh Khamesra  hitesh...@yahoo.com>> wrote:
>
> Current GeodeRedisAdapter implementation is based on
> https://cwiki.apache.org/confluence/display/GEODE/Geode+Redis+Adapter+Proposal
> .
> We are looking for some feedback on Redis commands and their mapping to
> geode region.
>
> 1. Redis Type String
>  a. Usage Set k1 v1
>  b. Current implementation creates "STRING_REGION" geode-partition-region
> upfront
>  c. This k1/v1 are geode-region key/value
>  d. Any feedback?
>
> 2. List Type
>  a. usage "rpush mylist A"
>  b. Current implementation maps each list to geode-partition-region(i.e.
> mylist is geode-partition-region); with the ability to get item from
> head/tail
>  c. Feedback/vote
>      -- List type operation at region-entry level;
>      -- region-key = "mylist"
>      -- region-value = Arraylist (will support all redis list ops)
>  d. Feedback/vote: both behavior is desirable
>
>
> 3. Hashes
>  a. this represents field-value or java bean object
>  b. usage "hmset user1000 username antirez birthyear 1977 verified 1"
>  c. Current implementation maps each hashes to
> geode-partition-region(i.e. user1000 is geode-partition-region)
>  d. Feedback/vote
>    -- Should we map hashes to region-entry
>    -- region-key = user1000
>    -- region-value = map
>    -- This will provide java bean sort to behaviour with 10s of
> field-value
>    -- Personally I would prefer this..
>  e. Feedback/vote: both behaviour is desirable
>
> 4. Sets
>  a. This represents unique keys in set
>  b. usage "sadd myset 1 2 3"
>  c. Current implementation maps each sadd to geode-partition-region(i.e.
> myset is geode-partition-region)
>  d. Feedback/vote
>    -- Should we map set to region-entry
>    -- region-key = myset
>    -- region-value = Hashset
>  e. Feedback/vote: both behaviour is desirable
>
> 5. SortedSets
>  a. This represents unique keys in set with score (usecase Query top-10)
>  b. usage "zadd hackers 1940 "Alan Kay""
>  c. Current implementation maps each zadd to geode-partition-region(i.e.
> hackers is geode-partition-regio

[ANNOUNCE] Apache Geode release 1.1.0

[Hitesh Khamesra]

The Apache Geode team is proud to announce Apache Geode release version 1.1.0

Apache *Geode* is a data management platform that provides a
database-like consistency model, reliable transaction processing and a
shared-nothing architecture to maintain very low latency performance with
high concurrency processing.

The release artifacts are available at:
  http://geode.apache.org/releases/

To use the artifacts, please use the following documentation:
  http://geode.apache.org/docs/guide/11/about_geode.html

Git source:
  
https://git-wip-us.apache.org/repos/asf?p=geode.git;a=tag;h=1d7a03bbab47a154b59dc6a4e811f7fc4a74c941

Git commit SHA1: 2286fd064a52173eab8fdcfadfb89a01e81ef728

Release Notes:
  
https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.1.0

We would like to thank all the contributors that made the release possible.

Regards,
Hitesh & Karen on behalf of the Apache Geode team

Re: GeodeRedisAdapter improvments/feedback

[Hitesh Khamesra]

Thanks for all your feedback.
Dan, your third option looks good ( cost of memory/oql need to consider). But 
this should solves popular redis  use cases "Top k/ multiple leaderboards use 
cases, where subset of users/entity involves".
In general, Redis brings beautiful user apis which is very handful to end user. 
They don't need to think about distributed nature of stores.
Geode brings strong consistency, high availability(in few seconds secondary 
copy is available), wan-replication, Async queue and many more enterprise 
features.

Thus providing those apis on top of Geode should generate good value. 

Thanks Again.Hitesh.



  From: Dan Smith 
 To: dev@geode.apache.org 
 Sent: Wednesday, February 15, 2017 3:12 PM
 Subject: Re: GeodeRedisAdapter improvments/feedback
   
Deltas would help. If the regions are persistent, the whole new value does
get written to disk though.

I suppose a third option would be to store all collections in the same
region, but store each element of the collection as a separate entry. For
example for HSET rkey rfield rvalue would create an entry with a geode key
of (rkey, rfield) in a REDIS_MAPS region. We could index the based on rkey.
That might make operations on the entire collection more expensive though;
those operations would all require using OQL queries.

-Dan



On Wed, Feb 15, 2017 at 2:35 PM, Galen M O'Sullivan 
wrote:

> On Wed, Feb 15, 2017 at 1:34 PM, Real Wes  wrote:
> > Does delta propagation make worrying about frequently updated fat
> collections moot?
>
> As long as we don't have collections on the order of the available RAM on a
> single server (in which case we'd want to distribute it across multiple
> servers), and pending testing, I think it does. Good idea, I didn't even
> know we could do this.
>


   

Re: for discussion: separate website into its own repo+1

[Hitesh Khamesra]

+1

Sent from Yahoo Mail on Android 
 
  On Thu, Feb 16, 2017 at 5:06 PM, Anthony Baker wrote:   
Yes, please.  Let’s call the repo geode-site.  Use two branches:  master and 
asf-site.  If we can auto-build and push to asf-site that would be awesome.

Anthony

> On Feb 16, 2017, at 4:38 PM, Dan Smith  wrote:
> 
> +1
> 
> I think the current setup is confusing, because the website is supposed to
> include docs that are generated from the last release, but the site
> instructions say the site should be generated from develop. A separate repo
> with a single branch will probably reduce confusion.
> 
> We also need to script the website building and publishing, and ideally
> have the publishing done by a CI system based on commits. It looks like
> some other projects are talking about doing this with jenkins jenkins - see
> INFRA-10722 for example.
> 
> -Dan
> 
> On Thu, Feb 16, 2017 at 4:10 PM, Karen Miller  wrote:
> 
>> I think that the website content that is currently in geode/geode-site
>> ought to be moved to its own repository.  The driving reason for this is
>> that changes to the website occur on a different schedule than code
>> releases.  We often want to add a new committer's name or a new
>> event, and these items are not associated with sw releases. A new website
>> release that comes from the develop branch may have commits that
>> should not yet be made public.
>> 
>> Are there downsides to separating the website content into its own repo?
>> 
  

Re: GeodeRedisAdapter improvments/feedback

[Hitesh Khamesra]

Right..

Sent from Yahoo Mail on Android 
 
  On Fri, Feb 17, 2017 at 8:05 AM, Wes Williams wrote:   
I'm not clear on the reference to "I like the idea of first class data
structures like Lists and Sorted Sets."

Is the suggestion here to extend Geode to not only support a distributed
ConcurrentHashMap but also distributed ConcurrentList's and
ConcurrentSortedSet's?


*Wes Williams | Pivotal Advisory **Data Engineer*
781.606.0325
http://pivotal.io/big-data/pivotal-gemfire

On Thu, Feb 16, 2017 at 10:34 AM, Michael Stolz  wrote:

> I like the idea of first class data structures like Lists and Sorted Sets.
>
> I'm not sure which side of the fence I'm on in terms of very large objects
> and using Regions to represent them. Feels very heavy because of all the
> overhead of a Region entry in Geode (over 300 bytes per entry).
>
> I think the main reason people will want to use Geode in place of Redis
> will be horizontal scale in terms of the number of structures first, size
> of structures second, ability to get additional enterprise features like
> synchronous instead of asynchronous replication from masters to slaves
> (zero-data-loss) multi-site and even multi-cloud use cases (WAN Gateway).
>
>
>
> --
> Mike Stolz
> Principal Engineer, GemFire Product Manager
> Mobile: +1-631-835-4771
>
> On Wed, Feb 15, 2017 at 8:09 PM, Swapnil Bawaskar 
> wrote:
>
> > I think we as a community need to determine what value do we want to add
> > with the Redis adapter. Redis already does a great job storing small data
> > structures in memory and sharding them. We do a great job of making sure
> > that these data structures are horizontally scalable; why would we want
> to
> > replicate what another open source project is already implementing?
> >
> > Having said that, I would like to see a configuration property that lets
> > the user chose between a single server vs a distributed collection.
> >
> >
> > > I think we could have the following options:
> > >
> > >  1. have a property that could be set to use either single server
> > >    collections over use the current distributed collection
> > >  2. have first class collection implementations that are distributed by
> > >    nature, as using key:value as the hammer for all does not make
> sense
> > >
> >
> > I don't think these options are mutually exclusive. We should make lists
> > and SortedSets first class data structures in Geode alongside regions.
> >
>
  

Re: [GitHub] geode pull request #404: Geode 2469

[Hitesh Khamesra]

Thanks Gregory.  I will look this further.


  From: Gregory Green 
 To: dev@geode.apache.org 
 Sent: Tuesday, February 21, 2017 12:21 PM
 Subject: Re: [GitHub] geode pull request #404: Geode 2469
   
Hello Everyone,

I just wanted to clarify something with this pull request.

The main benefit of the pull request changes is the 99.9% performance
improvements for SET and HASH related Redis command processing.

FYI - In order to support HA as has previously implemented, the default
region type has to be set as a type that supports HA such as
PARTITION_REDUNDANT,
REPLICATE, etc.

If administrators start the adapter to indicate an HA default data type as
the following.

start server --name=server1 --redis-bind-address=localhost --redis-port=11211
--J=-Dgemfireredis.regiontype=PARTITION_REDUNDANT.

The default type if not set is PARTITION, which as no HA support.

This change does not fundamentally change the way the Redis adapter handles
the created regions, but it does limit the number of regions created. The
new pull request creates two new regions RediS_SET and RediS_HASH use the
same conventions used by the current adapter Redis_STRING, Redis_HLL, etc
used in Geode 1.1.0 and GemFire 9.0.1.

Previously, the Redis adapter created a region for each key provided in a
Redis SET or HASH related command. This convention causes issues when
non-standard region name characters such as ":" exists in the key. The main
problem is Redis uses ":" in its key to indicated the object that the key
belongs with the format "object:key". For example, HSET companies:1000
means the object name is companies and its key if 1000. The pull request
changes will only create a new region if Redis "object:key" convention is
used in the HASH keys (otherwise the RediS_HASH region is used). So HSET
companies:1000 ... will result in a new region companies region created
with an entry key=100. Administrators can pre-create the region (ex:
companies) with the desired attributes. If the region is not pre-created it
will be created on demand with using the default region type.

On Sat, Feb 18, 2017 at 11:22 PM, ggreen  wrote:

> GitHub user ggreen opened a pull request:
>
>    https://github.com/apache/geode/pull/404
>
>    Geode 2469
>
>    The updated Geode Redis Adapter now works with a sample Spring Data
> Redis Example
>    [GEODE-2469.pdf](https://github.com/apache/geode/files/
> 785580/GEODE-2469.pdf)
>
>    These changes are focused on the HASH and Set Redis Data Types to
> support Spring Data Redis sample code located at the following URL
>
>    https://github.com/Pivotal-Data-Engineering/gemfire9_
> examples/tree/person_example_sdg_Tracker139498217/redis/
> spring-data-redis-example/src/test/java/io/pivotal/redis/
> gemfire/example/repository
>
>    The Hash performance changes from this pull request had a 99.8%
> performance improvement.
>
>    This is based on the Hashes JUNIT Tests.
>
>    https://github.com/Pivotal-Data-Engineering/gemfire9_
> examples/blob/person_example_sdg_Tracker139498217/redis/
> gemfire-streaming-client/src/test/java/io/pivotal/gemfire9/
> HashesJUnitTest.java
>
>    This code executed in 12.549s against Gemfire 9.0.1 code. After the
> changes, the test executed in 0.022s with the GEODE-2469 pull request.
>
>    Redis Set related command performance had a 99.9% performance
> improvement.
>
>    See  https://github.com/Pivotal-Data-Engineering/gemfire9_
> examples/blob/person_example_sdg_Tracker139498217/redis/
> gemfire-streaming-client/src/test/java/io/pivotal/gemfire9/
> SetsJUnitTest.java
>
>    The previous Set Junit tests executed against GemFire 9.0.1 executed
> in 31.507 seconds. These same test executed in 0.036 seconds with the
> GEODE-2469 pull request changes.
>
>    The GemFire 9.0.1 Geode (1.1.0) version for the Geode Redis adapter
> created a Geode Region for each key provided in the Redis Hash or Set
> related command. Each Redis command to remove key entry previously
> destroyed the region. The big performance gain is based on using a new
> ReDiS_HASH and ReDiS_SET region. Note the changed will create or reuse an
> existing region with a matching name for Redis HASH commands references
> objects. For Redis HASH object's key will have a format of object:key
>
>    Please see https://redis.io/topics/data-types-intro HASH section on
> objects for information on Redis objects.
>
> You can merge this pull request into a Git repository by running:
>
>    $ git pull https://github.com/ggreen/geode GEODE-2469
>
> Alternatively you can review and apply these changes as the patch at:
>
>    https://github.com/apache/geode/pull/404.patch
>
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
>
>    This closes #404
>
> 
> commit 44b90c4f3f8e1ec4fdae63a8be126982d7c837a2
> Author: Gregory Green 
> Date:  2017-02-14T20:31:49Z
>
>    GEODE-2469 initial changes to support Redis objects
>
> commit 27d7600e85945a7134115630efe378ed

Re: Review Request 56986: GEODE-2497 surprise members are never timed out during startu

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56986/#review166579
---


Ship it!




Ship It!

- Hitesh Khamesra


On Feb. 23, 2017, 4:17 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56986/
> ---
> 
> (Updated Feb. 23, 2017, 4:17 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan and Hitesh Khamesra.
> 
> 
> Bugs: GEODE-2497
> https://issues.apache.org/jira/browse/GEODE-2497
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> fixing NPE hit by tests using disable-tcp=true.  In this configuration there 
> is no dclistener in the membership manager but there _is_ a listener.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  d0a0cbed9c895af26dfba9c5d5ca7fe50b195a30 
> 
> Diff: https://reviews.apache.org/r/56986/diff/
> 
> 
> Testing
> ---
> 
> precheckin, disable-tcp=true testing
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 56999: GEODE-2534 concurrently started locators fail to create a unified system

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56999/#review166616
---


Ship it!




Ship It!

- Hitesh Khamesra


On Feb. 23, 2017, 9:50 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56999/
> ---
> 
> (Updated Feb. 23, 2017, 9:50 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan and Hitesh Khamesra.
> 
> 
> Bugs: GEODE-2534
> https://issues.apache.org/jira/browse/GEODE-2534
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GMS services were being installed in the locator before they were started, 
> causing the locator to not know its own member ID.  This caused the 
> concurrent startup registry to not contain its member ID, allowing 
> FindCoordinatorResponses to be incorrect.
> 
> * locator A starts location service, installs GMS services w/o ID
> * locator B sends FindCoordinatorRequest to A, gets response saying B should 
> be coordinator
> * locator B starts to become coordinator
> * locator A establishes its ID, sends FindCoordinator request to A & B, gets 
> responses saying A should  be coordinator
> * locator A becomes coordinator with view [A]
> * locator B finishes becoming coordinator with view [B]
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/distributed/Locator.java 
> 2c7c251b3be09309819c4c413436cd47c70970be 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
>  6d7c5a91fdfbf43b7a9e6305a32f556390d05d3c 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
>  8d340070bdf6839b73217879427785502710c3bd 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  c05140d29fbdc26a1e218a9c393f3ba5bed81089 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> da0daeb5f85a7c3aeb484ce1db820b9a96dd2f2e 
> 
> Diff: https://reviews.apache.org/r/56999/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>