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/12/23 18:10:34 UTC

Review Request 55017: cleaning up code in a few places

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

Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.


Repository: geode


Description
-------

While investigating a couple of failures I ran across a few things that needed to be fixed.

1. EndpointManagerImpl was skipping notification of listeners if there was no member ID in an endpoint but the member ID is no longer used.  I removed the check.
2. During a Forced Disconnect CacheClientProxy was skipping a lot of clean-up because it was expecting a CacheClosedException instead of a CancelException.  We should never catch CacheClosedException - always catch CancelException.
3. When SSL is being used the P2P Reader Threads were not setting connection attributes in the thread's name like we do with non-SSL reader threads.  This made debugging an SSL failure a bit more difficult.


Diffs
-----

  geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java 6d5d9d689bab90895e8035b3055a2fb543b29f54 
  geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxy.java 6e31fe5488b64f9c5b4a4c867e72fc2c60e40591 
  geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java d44476f2f6f9387b390ea7e3758a057e43edef5a 

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


Testing
-------

precheckin, integration testing


Thanks,

Bruce Schuchardt


Re: Review Request 55017: cleaning up code in a few places

Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55017/#review160168
-----------------------------------------------------------



bump - please review this diff

- Bruce Schuchardt


On Dec. 23, 2016, 6:10 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55017/
> -----------------------------------------------------------
> 
> (Updated Dec. 23, 2016, 6:10 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> While investigating a couple of failures I ran across a few things that needed to be fixed.
> 
> 1. EndpointManagerImpl was skipping notification of listeners if there was no member ID in an endpoint but the member ID is no longer used.  I removed the check.
> 2. During a Forced Disconnect CacheClientProxy was skipping a lot of clean-up because it was expecting a CacheClosedException instead of a CancelException.  We should never catch CacheClosedException - always catch CancelException.
> 3. When SSL is being used the P2P Reader Threads were not setting connection attributes in the thread's name like we do with non-SSL reader threads.  This made debugging an SSL failure a bit more difficult.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java 6d5d9d689bab90895e8035b3055a2fb543b29f54 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxy.java 6e31fe5488b64f9c5b4a4c867e72fc2c60e40591 
>   geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java d44476f2f6f9387b390ea7e3758a057e43edef5a 
> 
> Diff: https://reviews.apache.org/r/55017/diff/
> 
> 
> Testing
> -------
> 
> precheckin, integration testing
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>


Re: Review Request 55017: cleaning up code in a few places

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Dec. 23, 2016, 6:10 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55017/
> -----------------------------------------------------------
> 
> (Updated Dec. 23, 2016, 6:10 p.m.)
> 
> 
> Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> While investigating a couple of failures I ran across a few things that needed to be fixed.
> 
> 1. EndpointManagerImpl was skipping notification of listeners if there was no member ID in an endpoint but the member ID is no longer used.  I removed the check.
> 2. During a Forced Disconnect CacheClientProxy was skipping a lot of clean-up because it was expecting a CacheClosedException instead of a CancelException.  We should never catch CacheClosedException - always catch CancelException.
> 3. When SSL is being used the P2P Reader Threads were not setting connection attributes in the thread's name like we do with non-SSL reader threads.  This made debugging an SSL failure a bit more difficult.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java 6d5d9d689bab90895e8035b3055a2fb543b29f54 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxy.java 6e31fe5488b64f9c5b4a4c867e72fc2c60e40591 
>   geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java d44476f2f6f9387b390ea7e3758a057e43edef5a 
> 
> Diff: https://reviews.apache.org/r/55017/diff/
> 
> 
> Testing
> -------
> 
> precheckin, integration testing
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>