You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Bruce Schuchardt <bs...@pivotal.io> on 2016/04/15 01:07:00 UTC

Review Request 46232: GEODE-1174 CI failure: UniversalMembershipListenerAdapterDUnitTest.testSystemClientEventsInServer

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

Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.


Repository: geode


Description
-------

When a client closes its connection Pool a CloseConnection request is sent to the server and then sockets are closed.  Unfortunately this sometimes results in the socket being closed before the operation reaches the server and the server sees the client as having crashed.  This is mostly a problem if the client is sending keepAlive==true but it is also what is causing this test to fail intermittently.

I've changed CloseConnectionOp to wait for a reply and have changed the server to send this reply.  It's possible for the client to get an EOFException instead of a reply, but at least this causes the client to wait for the server to terminate the connection so that we know that the server has received the request and acted on it.

Since old GemFire clients might be used with Geode I've added checks for them and avoid sending a reply in that case.


Diffs
-----

  geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/CloseConnectionOp.java cbfa3a6035f292f018f139018d342b3b74be8b8e 
  geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/CloseConnection.java 59e1ac43c5f16809de4ec1f659ecbbbffd7978e0 

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


Testing
-------


Thanks,

Bruce Schuchardt


Re: Review Request 46232: GEODE-1174 CI failure: UniversalMembershipListenerAdapterDUnitTest.testSystemClientEventsInServer

Posted by Udo Kohlmeyer <uk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46232/#review129832
-----------------------------------------------------------


Ship it!




Ship It!

- Udo Kohlmeyer


On April 14, 2016, 11:06 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46232/
> -----------------------------------------------------------
> 
> (Updated April 14, 2016, 11:06 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> When a client closes its connection Pool a CloseConnection request is sent to the server and then sockets are closed.  Unfortunately this sometimes results in the socket being closed before the operation reaches the server and the server sees the client as having crashed.  This is mostly a problem if the client is sending keepAlive==true but it is also what is causing this test to fail intermittently.
> 
> I've changed CloseConnectionOp to wait for a reply and have changed the server to send this reply.  It's possible for the client to get an EOFException instead of a reply, but at least this causes the client to wait for the server to terminate the connection so that we know that the server has received the request and acted on it.
> 
> Since old GemFire clients might be used with Geode I've added checks for them and avoid sending a reply in that case.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/CloseConnectionOp.java cbfa3a6035f292f018f139018d342b3b74be8b8e 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/CloseConnection.java 59e1ac43c5f16809de4ec1f659ecbbbffd7978e0 
> 
> Diff: https://reviews.apache.org/r/46232/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>


Re: Review Request 46232: GEODE-1174 CI failure: UniversalMembershipListenerAdapterDUnitTest.testSystemClientEventsInServer

Posted by Hitesh Khamesra <hk...@pivotal.io>.

> On April 15, 2016, 4:24 p.m., Hitesh Khamesra wrote:
> > I was trying to ignore this while fixing DurableClient issues. As it can increase client cache close time. We may want to track why client didn't send close message, i mean is there any thread which is just closing the connection without sending close message to server.Otherwise it look good.
> 
> Bruce Schuchardt wrote:
>     In this test the client uses PoolManager.close() to shut down the pool.  This method invokes close() on the ConnectionImpl objects and that method will send a CloseConnection message unless a ForcedDisconnect is shutting things down, which wasn't the case in the failures I've looked at.

There may be some other client thread like ping thread, connection-load-balance trhead(Background thread) etc..


- Hitesh


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


On April 14, 2016, 11:06 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46232/
> -----------------------------------------------------------
> 
> (Updated April 14, 2016, 11:06 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> When a client closes its connection Pool a CloseConnection request is sent to the server and then sockets are closed.  Unfortunately this sometimes results in the socket being closed before the operation reaches the server and the server sees the client as having crashed.  This is mostly a problem if the client is sending keepAlive==true but it is also what is causing this test to fail intermittently.
> 
> I've changed CloseConnectionOp to wait for a reply and have changed the server to send this reply.  It's possible for the client to get an EOFException instead of a reply, but at least this causes the client to wait for the server to terminate the connection so that we know that the server has received the request and acted on it.
> 
> Since old GemFire clients might be used with Geode I've added checks for them and avoid sending a reply in that case.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/CloseConnectionOp.java cbfa3a6035f292f018f139018d342b3b74be8b8e 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/CloseConnection.java 59e1ac43c5f16809de4ec1f659ecbbbffd7978e0 
> 
> Diff: https://reviews.apache.org/r/46232/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>


Re: Review Request 46232: GEODE-1174 CI failure: UniversalMembershipListenerAdapterDUnitTest.testSystemClientEventsInServer

Posted by Bruce Schuchardt <bs...@pivotal.io>.

> On April 15, 2016, 4:24 p.m., Hitesh Khamesra wrote:
> > I was trying to ignore this while fixing DurableClient issues. As it can increase client cache close time. We may want to track why client didn't send close message, i mean is there any thread which is just closing the connection without sending close message to server.Otherwise it look good.

In this test the client uses PoolManager.close() to shut down the pool.  This method invokes close() on the ConnectionImpl objects and that method will send a CloseConnection message unless a ForcedDisconnect is shutting things down, which wasn't the case in the failures I've looked at.


- Bruce


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


On April 14, 2016, 11:06 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46232/
> -----------------------------------------------------------
> 
> (Updated April 14, 2016, 11:06 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> When a client closes its connection Pool a CloseConnection request is sent to the server and then sockets are closed.  Unfortunately this sometimes results in the socket being closed before the operation reaches the server and the server sees the client as having crashed.  This is mostly a problem if the client is sending keepAlive==true but it is also what is causing this test to fail intermittently.
> 
> I've changed CloseConnectionOp to wait for a reply and have changed the server to send this reply.  It's possible for the client to get an EOFException instead of a reply, but at least this causes the client to wait for the server to terminate the connection so that we know that the server has received the request and acted on it.
> 
> Since old GemFire clients might be used with Geode I've added checks for them and avoid sending a reply in that case.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/CloseConnectionOp.java cbfa3a6035f292f018f139018d342b3b74be8b8e 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/CloseConnection.java 59e1ac43c5f16809de4ec1f659ecbbbffd7978e0 
> 
> Diff: https://reviews.apache.org/r/46232/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>


Re: Review Request 46232: GEODE-1174 CI failure: UniversalMembershipListenerAdapterDUnitTest.testSystemClientEventsInServer

Posted by Hitesh Khamesra <hk...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46232/#review129137
-----------------------------------------------------------


Ship it!




I was trying to ignore this while fixing DurableClient issues. As it can increase client cache close time. We may want to track why client didn't send close message, i mean is there any thread which is just closing the connection without sending close message to server.Otherwise it look good.

- Hitesh Khamesra


On April 14, 2016, 11:06 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46232/
> -----------------------------------------------------------
> 
> (Updated April 14, 2016, 11:06 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> When a client closes its connection Pool a CloseConnection request is sent to the server and then sockets are closed.  Unfortunately this sometimes results in the socket being closed before the operation reaches the server and the server sees the client as having crashed.  This is mostly a problem if the client is sending keepAlive==true but it is also what is causing this test to fail intermittently.
> 
> I've changed CloseConnectionOp to wait for a reply and have changed the server to send this reply.  It's possible for the client to get an EOFException instead of a reply, but at least this causes the client to wait for the server to terminate the connection so that we know that the server has received the request and acted on it.
> 
> Since old GemFire clients might be used with Geode I've added checks for them and avoid sending a reply in that case.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/CloseConnectionOp.java cbfa3a6035f292f018f139018d342b3b74be8b8e 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/CloseConnection.java 59e1ac43c5f16809de4ec1f659ecbbbffd7978e0 
> 
> Diff: https://reviews.apache.org/r/46232/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>