You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Darrel Schneider <ds...@pivotal.io> on 2015/10/13 19:07:28 UTC
Review Request 39279: GEODE-397: Fix clients to use server ssl config
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39279/
-----------------------------------------------------------
Review request for geode, Barry Oglesby and Jacob Barrett.
Bugs: GEODE-397
https://issues.apache.org/jira/browse/GEODE-397
Repository: geode
Description
-------
Each client pool now creates a single SocketCreator that
uses either the server or gateway ssl config.
That SocketCreator is used for all connections the client
makes to the server. It no longer uses the default cluster
SocketCreator when connecting to the server.
This fix might show some performance improvement because the
old code recreated the SocketCreator every time the client
created a server connection. Now it just happens once for each
pool.
Also since using SocketCreator.getDefaultInstance when it should
have used a non-default instance caused this bug all calls of
getDefaultInstance were reviewed. A number of them were used
to call isHostReachable which is a method that was deadcoded.
So all those calls have been commented out. One of call of
getDefaultInstance was deleted (in ConnectionTable) because it
was never used.
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionFactoryImpl.java 02a1fc0f00a23a602fd08646685e3db2f0a86ad9
gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 749a765b5311df7cc910e3024753934b48206fae
gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 940936fcb8601c880ea6ff588d9d1f0366de0664
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 07dc030d083d8fc65c49dd448373a5b40b057470
gemfire-core/src/main/java/com/gemstone/gemfire/internal/tcp/ConnectionTable.java 508eba20a403b3ad2e27c2a646446a7472db62ea
gemfire-jgroups/src/main/java/com/gemstone/org/jgroups/stack/GossipClient.java 304bd529e5c05f596a9d6577c9cba6da1798e992
Diff: https://reviews.apache.org/r/39279/diff/
Testing
-------
Thanks,
Darrel Schneider
Re: Review Request 39279: GEODE-397: Fix clients to use server ssl
config
Posted by Barry Oglesby <bo...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39279/#review102492
-----------------------------------------------------------
Ship it!
Ship It!
- Barry Oglesby
On Oct. 13, 2015, 5:07 p.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39279/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2015, 5:07 p.m.)
>
>
> Review request for geode, Barry Oglesby and Jacob Barrett.
>
>
> Bugs: GEODE-397
> https://issues.apache.org/jira/browse/GEODE-397
>
>
> Repository: geode
>
>
> Description
> -------
>
> Each client pool now creates a single SocketCreator that
> uses either the server or gateway ssl config.
> That SocketCreator is used for all connections the client
> makes to the server. It no longer uses the default cluster
> SocketCreator when connecting to the server.
> This fix might show some performance improvement because the
> old code recreated the SocketCreator every time the client
> created a server connection. Now it just happens once for each
> pool.
>
> Also since using SocketCreator.getDefaultInstance when it should
> have used a non-default instance caused this bug all calls of
> getDefaultInstance were reviewed. A number of them were used
> to call isHostReachable which is a method that was deadcoded.
> So all those calls have been commented out. One of call of
> getDefaultInstance was deleted (in ConnectionTable) because it
> was never used.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionFactoryImpl.java 02a1fc0f00a23a602fd08646685e3db2f0a86ad9
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 749a765b5311df7cc910e3024753934b48206fae
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 940936fcb8601c880ea6ff588d9d1f0366de0664
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 07dc030d083d8fc65c49dd448373a5b40b057470
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/tcp/ConnectionTable.java 508eba20a403b3ad2e27c2a646446a7472db62ea
> gemfire-jgroups/src/main/java/com/gemstone/org/jgroups/stack/GossipClient.java 304bd529e5c05f596a9d6577c9cba6da1798e992
>
> Diff: https://reviews.apache.org/r/39279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 39279: GEODE-397: Fix clients to use server ssl
config
Posted by Jacob Barrett <jb...@amduat.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39279/#review102493
-----------------------------------------------------------
Ship it!
Ship It!
- Jacob Barrett
On Oct. 13, 2015, 10:07 a.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39279/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2015, 10:07 a.m.)
>
>
> Review request for geode, Barry Oglesby and Jacob Barrett.
>
>
> Bugs: GEODE-397
> https://issues.apache.org/jira/browse/GEODE-397
>
>
> Repository: geode
>
>
> Description
> -------
>
> Each client pool now creates a single SocketCreator that
> uses either the server or gateway ssl config.
> That SocketCreator is used for all connections the client
> makes to the server. It no longer uses the default cluster
> SocketCreator when connecting to the server.
> This fix might show some performance improvement because the
> old code recreated the SocketCreator every time the client
> created a server connection. Now it just happens once for each
> pool.
>
> Also since using SocketCreator.getDefaultInstance when it should
> have used a non-default instance caused this bug all calls of
> getDefaultInstance were reviewed. A number of them were used
> to call isHostReachable which is a method that was deadcoded.
> So all those calls have been commented out. One of call of
> getDefaultInstance was deleted (in ConnectionTable) because it
> was never used.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionFactoryImpl.java 02a1fc0f00a23a602fd08646685e3db2f0a86ad9
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 749a765b5311df7cc910e3024753934b48206fae
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 940936fcb8601c880ea6ff588d9d1f0366de0664
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 07dc030d083d8fc65c49dd448373a5b40b057470
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/tcp/ConnectionTable.java 508eba20a403b3ad2e27c2a646446a7472db62ea
> gemfire-jgroups/src/main/java/com/gemstone/org/jgroups/stack/GossipClient.java 304bd529e5c05f596a9d6577c9cba6da1798e992
>
> Diff: https://reviews.apache.org/r/39279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 39279: GEODE-397: Fix clients to use server ssl
config
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39279/#review102502
-----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java (line 1040)
<https://reviews.apache.org/r/39279/#comment160181>
SocketCreator now implements the SockCreator interface. So isHostReachable needs to be removed
from SockCreator and all implementations.
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java (line 313)
<https://reviews.apache.org/r/39279/#comment160182>
All these deadcoded calls of isHostReachable should just be removed from develop.
- Darrel Schneider
On Oct. 13, 2015, 10:07 a.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39279/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2015, 10:07 a.m.)
>
>
> Review request for geode, Barry Oglesby and Jacob Barrett.
>
>
> Bugs: GEODE-397
> https://issues.apache.org/jira/browse/GEODE-397
>
>
> Repository: geode
>
>
> Description
> -------
>
> Each client pool now creates a single SocketCreator that
> uses either the server or gateway ssl config.
> That SocketCreator is used for all connections the client
> makes to the server. It no longer uses the default cluster
> SocketCreator when connecting to the server.
> This fix might show some performance improvement because the
> old code recreated the SocketCreator every time the client
> created a server connection. Now it just happens once for each
> pool.
>
> Also since using SocketCreator.getDefaultInstance when it should
> have used a non-default instance caused this bug all calls of
> getDefaultInstance were reviewed. A number of them were used
> to call isHostReachable which is a method that was deadcoded.
> So all those calls have been commented out. One of call of
> getDefaultInstance was deleted (in ConnectionTable) because it
> was never used.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionFactoryImpl.java 02a1fc0f00a23a602fd08646685e3db2f0a86ad9
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 749a765b5311df7cc910e3024753934b48206fae
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 940936fcb8601c880ea6ff588d9d1f0366de0664
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 07dc030d083d8fc65c49dd448373a5b40b057470
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/tcp/ConnectionTable.java 508eba20a403b3ad2e27c2a646446a7472db62ea
> gemfire-jgroups/src/main/java/com/gemstone/org/jgroups/stack/GossipClient.java 304bd529e5c05f596a9d6577c9cba6da1798e992
>
> Diff: https://reviews.apache.org/r/39279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 39279: GEODE-397: Fix clients to use server ssl
config
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39279/#review102504
-----------------------------------------------------------
A test that reproduces this issue needs to be added.
- Darrel Schneider
On Oct. 13, 2015, 10:07 a.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39279/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2015, 10:07 a.m.)
>
>
> Review request for geode, Barry Oglesby and Jacob Barrett.
>
>
> Bugs: GEODE-397
> https://issues.apache.org/jira/browse/GEODE-397
>
>
> Repository: geode
>
>
> Description
> -------
>
> Each client pool now creates a single SocketCreator that
> uses either the server or gateway ssl config.
> That SocketCreator is used for all connections the client
> makes to the server. It no longer uses the default cluster
> SocketCreator when connecting to the server.
> This fix might show some performance improvement because the
> old code recreated the SocketCreator every time the client
> created a server connection. Now it just happens once for each
> pool.
>
> Also since using SocketCreator.getDefaultInstance when it should
> have used a non-default instance caused this bug all calls of
> getDefaultInstance were reviewed. A number of them were used
> to call isHostReachable which is a method that was deadcoded.
> So all those calls have been commented out. One of call of
> getDefaultInstance was deleted (in ConnectionTable) because it
> was never used.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionFactoryImpl.java 02a1fc0f00a23a602fd08646685e3db2f0a86ad9
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 749a765b5311df7cc910e3024753934b48206fae
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 940936fcb8601c880ea6ff588d9d1f0366de0664
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 07dc030d083d8fc65c49dd448373a5b40b057470
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/tcp/ConnectionTable.java 508eba20a403b3ad2e27c2a646446a7472db62ea
> gemfire-jgroups/src/main/java/com/gemstone/org/jgroups/stack/GossipClient.java 304bd529e5c05f596a9d6577c9cba6da1798e992
>
> Diff: https://reviews.apache.org/r/39279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darrel Schneider
>
>