You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by anilkumar gingade <ag...@pivotal.io> on 2016/11/02 23:41:28 UTC
Review Request 53410: GEODE-2064 Added check for system shutdown while
handlling connect exception.
-----------------------------------------------------------
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
Re: Review Request 53410: GEODE-2064 Added check for system shutdown
while handlling connect exception.
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/53410/#comment224447>
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
>
>
Re: Review Request 53410: GEODE-2064 Added check for system shutdown
while handlling connect exception.
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
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.
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
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
>
>