Re: Review Request 53410: GEODE-2064 Added check for system shutdown while handlling connect exception.

[Bruce Schuchardt]

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



That looks okay but since it's changing shutdown behavior please run 
splitBrain/splitBrain.bt and splitBrain/networkPartition3Hosts.bt

- Bruce Schuchardt


On Nov. 2, 2016, 11:41 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53410/
> ---
> 
> (Updated Nov. 2, 2016, 11:41 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, Eric Shu, Scott 
> Jewell, Ken Howe, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> While message send in progress, if the system gets shutdown (forced 
> disconnect), the send (message delivery to peers) reports connect exception 
> and ignores detecting/throwing SystemDisconnect exception. 
> 
> In "DirectChannel.getConnection()" it checks for "mgr.shutdownInProgress()" 
> and returns ConnectException to the caller 
> "GMSMembershipManager.directChannelSend()"
> 
> In client/server scenario, if the client is performing cache operation, the 
> cache operation may succeed in server that is getting down and failure to 
> deliver the message to other peers/servers. The client will see the operation 
> getting successfully completed.
> 
> The above scenario could result in client missing an event and resulting in 
> data mismatch between client and server.
> 
> Made changes to throw "DistributedSystemDisconnectedException" if system is 
> shutting down. This will result in caller to retry the operation.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  a4691f4 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManagerJUnitTest.java
>  bae1ddc 
> 
> Diff: https://reviews.apache.org/r/53410/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test. Verified the test without my change and with the change. 
> With change test looks for DistributedSystemDisconnectedException to be 
> thrown.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 53410: GEODE-2064 Added check for system shutdown while handlling connect exception.

[Darrel Schneider]

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


Ship it!




Ship It!

- Darrel Schneider


On Nov. 2, 2016, 4:41 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53410/
> ---
> 
> (Updated Nov. 2, 2016, 4:41 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, Eric Shu, Scott 
> Jewell, Ken Howe, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> While message send in progress, if the system gets shutdown (forced 
> disconnect), the send (message delivery to peers) reports connect exception 
> and ignores detecting/throwing SystemDisconnect exception. 
> 
> In "DirectChannel.getConnection()" it checks for "mgr.shutdownInProgress()" 
> and returns ConnectException to the caller 
> "GMSMembershipManager.directChannelSend()"
> 
> In client/server scenario, if the client is performing cache operation, the 
> cache operation may succeed in server that is getting down and failure to 
> deliver the message to other peers/servers. The client will see the operation 
> getting successfully completed.
> 
> The above scenario could result in client missing an event and resulting in 
> data mismatch between client and server.
> 
> Made changes to throw "DistributedSystemDisconnectedException" if system is 
> shutting down. This will result in caller to retry the operation.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  a4691f4 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManagerJUnitTest.java
>  bae1ddc 
> 
> Diff: https://reviews.apache.org/r/53410/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test. Verified the test without my change and with the change. 
> With change test looks for DistributedSystemDisconnectedException to be 
> thrown.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 53410: GEODE-2064 Added check for system shutdown while handlling connect exception.

[Darrel Schneider]

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




geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
 (line 1707)


I noticed that lines 1707-1711 are duplicated in directChannelSend lines 
1698-1702. Consider refactoring this code into a single method.


- Darrel Schneider


On Nov. 2, 2016, 4:41 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53410/
> ---
> 
> (Updated Nov. 2, 2016, 4:41 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, Eric Shu, Scott 
> Jewell, Ken Howe, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> While message send in progress, if the system gets shutdown (forced 
> disconnect), the send (message delivery to peers) reports connect exception 
> and ignores detecting/throwing SystemDisconnect exception. 
> 
> In "DirectChannel.getConnection()" it checks for "mgr.shutdownInProgress()" 
> and returns ConnectException to the caller 
> "GMSMembershipManager.directChannelSend()"
> 
> In client/server scenario, if the client is performing cache operation, the 
> cache operation may succeed in server that is getting down and failure to 
> deliver the message to other peers/servers. The client will see the operation 
> getting successfully completed.
> 
> The above scenario could result in client missing an event and resulting in 
> data mismatch between client and server.
> 
> Made changes to throw "DistributedSystemDisconnectedException" if system is 
> shutting down. This will result in caller to retry the operation.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  a4691f4 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManagerJUnitTest.java
>  bae1ddc 
> 
> Diff: https://reviews.apache.org/r/53410/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test. Verified the test without my change and with the change. 
> With change test looks for DistributedSystemDisconnectedException to be 
> thrown.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Review Request 53410: GEODE-2064 Added check for system shutdown while handlling connect exception.

[anilkumar gingade]

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

Review request for geode, Bruce Schuchardt, Darrel Schneider, Eric Shu, Scott 
Jewell, Ken Howe, and Swapnil Bawaskar.


Repository: geode


Description
---

While message send in progress, if the system gets shutdown (forced 
disconnect), the send (message delivery to peers) reports connect exception and 
ignores detecting/throwing SystemDisconnect exception. 

In "DirectChannel.getConnection()" it checks for "mgr.shutdownInProgress()" and 
returns ConnectException to the caller 
"GMSMembershipManager.directChannelSend()"

In client/server scenario, if the client is performing cache operation, the 
cache operation may succeed in server that is getting down and failure to 
deliver the message to other peers/servers. The client will see the operation 
getting successfully completed.

The above scenario could result in client missing an event and resulting in 
data mismatch between client and server.

Made changes to throw "DistributedSystemDisconnectedException" if system is 
shutting down. This will result in caller to retry the operation.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
 a4691f4 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManagerJUnitTest.java
 bae1ddc 

Diff: https://reviews.apache.org/r/53410/diff/


Testing
---

Added new unit test. Verified the test without my change and with the change. 
With change test looks for DistributedSystemDisconnectedException to be thrown.


Thanks,

anilkumar gingade