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/13 19:48:59 UTC
Review Request 46156: GEODE-679 Explore removing SocketIOWithTimeout
and other classes related to FD soft leak
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46156/
-----------------------------------------------------------
Review request for geode, Hitesh Khamesra, Jason Huynh, Jianxia Chen, and Udo Kohlmeyer.
Bugs: GEODE-679
https://issues.apache.org/jira/browse/GEODE-679
Repository: geode
Description
-------
SocketUtils was added to avoid a file descriptor "leak" caused by the use of NIO socket channel selectors. This was spurred by a HADOOP JIRA ticket that claimed that sun.misc.Cleaners were being used to close selectors (see https://issues.apache.org/jira/browse/HADOOP-4346). I have verified that Cleaners are no longer used to close selectors and that SocketUtils is not making any difference in the number of file descriptors created in servers using NIO selectors using the (recently deleted) FDDUnitTest with modifications to force clients to close their connections to the server.
So, this change-set removes SocketUtils, reverting all made to introduce it in GemFire 8.0 to use direct method invokations on sockets.
Diffs
-----
geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 5016d67a7162268e2c793152f702d1f4d60f71d3
geode-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 2130a261a395b19a3b3aa197d9f97eab3cab2cc0
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java 5b20e8634328df490ac845c57fd35837890d5ae3
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java 3178b8d1de4d649597d429e651a3b06877347e52
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 3e43b69398725d32cd393c5da90c0f6969282ae3
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java 2ea6ca06c38de54eb9ac1184995c7908424eaa55
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java bfe382c0c5aac7e6286a4f7841f9b50724fe59db
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerConnection.java 1dd2562dbcda007e90908c7e21714b26f5b9bbe7
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerHandShakeProcessor.java 07185b8cfc6c985b856f497a79ccb37af239a23a
geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java bf44cd379f68d07edac78fe44b045a1e88f5c5f5
Diff: https://reviews.apache.org/r/46156/diff/
Testing
-------
precheckin is underway
Thanks,
Bruce Schuchardt
Re: Review Request 46156: GEODE-679 Explore removing
SocketIOWithTimeout and other classes related to FD soft leak
Posted by Bruce Schuchardt <bs...@pivotal.io>.
> On April 13, 2016, 6:01 p.m., Jason Huynh wrote:
> > I think we can also delete SocketIOWithTimeout, SocketInputStream and SocketOutputStream along with these changes
I'll remove those and SocketInputWrapper as well. There is only one reference to these classes, in GemFireCacheImpl.close, so I don't think I'll update the diff.
- Bruce
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46156/#review128717
-----------------------------------------------------------
On April 13, 2016, 5:48 p.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46156/
> -----------------------------------------------------------
>
> (Updated April 13, 2016, 5:48 p.m.)
>
>
> Review request for geode, Hitesh Khamesra, Jason Huynh, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Bugs: GEODE-679
> https://issues.apache.org/jira/browse/GEODE-679
>
>
> Repository: geode
>
>
> Description
> -------
>
> SocketUtils was added to avoid a file descriptor "leak" caused by the use of NIO socket channel selectors. This was spurred by a HADOOP JIRA ticket that claimed that sun.misc.Cleaners were being used to close selectors (see https://issues.apache.org/jira/browse/HADOOP-4346). I have verified that Cleaners are no longer used to close selectors and that SocketUtils is not making any difference in the number of file descriptors created in servers using NIO selectors using the (recently deleted) FDDUnitTest with modifications to force clients to close their connections to the server.
>
> So, this change-set removes SocketUtils, reverting all made to introduce it in GemFire 8.0 to use direct method invokations on sockets.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 5016d67a7162268e2c793152f702d1f4d60f71d3
> geode-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 2130a261a395b19a3b3aa197d9f97eab3cab2cc0
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java 5b20e8634328df490ac845c57fd35837890d5ae3
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java 3178b8d1de4d649597d429e651a3b06877347e52
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 3e43b69398725d32cd393c5da90c0f6969282ae3
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java 2ea6ca06c38de54eb9ac1184995c7908424eaa55
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java bfe382c0c5aac7e6286a4f7841f9b50724fe59db
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerConnection.java 1dd2562dbcda007e90908c7e21714b26f5b9bbe7
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerHandShakeProcessor.java 07185b8cfc6c985b856f497a79ccb37af239a23a
> geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java bf44cd379f68d07edac78fe44b045a1e88f5c5f5
>
> Diff: https://reviews.apache.org/r/46156/diff/
>
>
> Testing
> -------
>
> precheckin is underway
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 46156: GEODE-679 Explore removing
SocketIOWithTimeout and other classes related to FD soft leak
Posted by Jason Huynh <hu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46156/#review128717
-----------------------------------------------------------
I think we can also delete SocketIOWithTimeout, SocketInputStream and SocketOutputStream along with these changes
- Jason Huynh
On April 13, 2016, 5:48 p.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46156/
> -----------------------------------------------------------
>
> (Updated April 13, 2016, 5:48 p.m.)
>
>
> Review request for geode, Hitesh Khamesra, Jason Huynh, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Bugs: GEODE-679
> https://issues.apache.org/jira/browse/GEODE-679
>
>
> Repository: geode
>
>
> Description
> -------
>
> SocketUtils was added to avoid a file descriptor "leak" caused by the use of NIO socket channel selectors. This was spurred by a HADOOP JIRA ticket that claimed that sun.misc.Cleaners were being used to close selectors (see https://issues.apache.org/jira/browse/HADOOP-4346). I have verified that Cleaners are no longer used to close selectors and that SocketUtils is not making any difference in the number of file descriptors created in servers using NIO selectors using the (recently deleted) FDDUnitTest with modifications to force clients to close their connections to the server.
>
> So, this change-set removes SocketUtils, reverting all made to introduce it in GemFire 8.0 to use direct method invokations on sockets.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 5016d67a7162268e2c793152f702d1f4d60f71d3
> geode-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 2130a261a395b19a3b3aa197d9f97eab3cab2cc0
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java 5b20e8634328df490ac845c57fd35837890d5ae3
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java 3178b8d1de4d649597d429e651a3b06877347e52
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 3e43b69398725d32cd393c5da90c0f6969282ae3
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java 2ea6ca06c38de54eb9ac1184995c7908424eaa55
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java bfe382c0c5aac7e6286a4f7841f9b50724fe59db
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerConnection.java 1dd2562dbcda007e90908c7e21714b26f5b9bbe7
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerHandShakeProcessor.java 07185b8cfc6c985b856f497a79ccb37af239a23a
> geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java bf44cd379f68d07edac78fe44b045a1e88f5c5f5
>
> Diff: https://reviews.apache.org/r/46156/diff/
>
>
> Testing
> -------
>
> precheckin is underway
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 46156: GEODE-679 Explore removing
SocketIOWithTimeout and other classes related to FD soft leak
Posted by Jianxia Chen <jc...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46156/#review128805
-----------------------------------------------------------
SocketUtils is not in the diff. I assume you will remove both SocketUtils classes.
- Jianxia Chen
On April 13, 2016, 5:48 p.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46156/
> -----------------------------------------------------------
>
> (Updated April 13, 2016, 5:48 p.m.)
>
>
> Review request for geode, Hitesh Khamesra, Jason Huynh, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Bugs: GEODE-679
> https://issues.apache.org/jira/browse/GEODE-679
>
>
> Repository: geode
>
>
> Description
> -------
>
> SocketUtils was added to avoid a file descriptor "leak" caused by the use of NIO socket channel selectors. This was spurred by a HADOOP JIRA ticket that claimed that sun.misc.Cleaners were being used to close selectors (see https://issues.apache.org/jira/browse/HADOOP-4346). I have verified that Cleaners are no longer used to close selectors and that SocketUtils is not making any difference in the number of file descriptors created in servers using NIO selectors using the (recently deleted) FDDUnitTest with modifications to force clients to close their connections to the server.
>
> So, this change-set removes SocketUtils, reverting all made to introduce it in GemFire 8.0 to use direct method invokations on sockets.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java 5016d67a7162268e2c793152f702d1f4d60f71d3
> geode-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 2130a261a395b19a3b3aa197d9f97eab3cab2cc0
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java 5b20e8634328df490ac845c57fd35837890d5ae3
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java 3178b8d1de4d649597d429e651a3b06877347e52
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java 3e43b69398725d32cd393c5da90c0f6969282ae3
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java 2ea6ca06c38de54eb9ac1184995c7908424eaa55
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java bfe382c0c5aac7e6286a4f7841f9b50724fe59db
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerConnection.java 1dd2562dbcda007e90908c7e21714b26f5b9bbe7
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerHandShakeProcessor.java 07185b8cfc6c985b856f497a79ccb37af239a23a
> geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java bf44cd379f68d07edac78fe44b045a1e88f5c5f5
>
> Diff: https://reviews.apache.org/r/46156/diff/
>
>
> Testing
> -------
>
> precheckin is underway
>
>
> Thanks,
>
> Bruce Schuchardt
>
>