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/01/28 01:24:20 UTC

Review Request 42883: GEODE-871: Frequent use of executeOnAllServers causes build-up of sockets in TIME_WAIT on locator machine

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

Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.


Bugs: GEODE-871
    https://issues.apache.org/jira/browse/GEODE-871


Repository: geode


Description
-------

This change-set alters the client to abort its TCP/IP connection to the locator by enabling SO_LINGER and setting the timeout to zero before it closes the connection.  The Locator closes its connection first, which puts it into TIME_WAIT.  The client then aborts the connection, which cleans up the Locator's TIME_WAIT connection.


Diffs
-----

  gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java 47f50b342e495a25978d86f717b6140b67687618 
  gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpServer.java f52b9ab3a47c9dc4147a852695b789a2d11b958e 

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


Testing
-------

Manual testing using TcpClient to send GetAllServersRequests to the locator thousands of times & counting the number of sockets open on the locator machine with "netstat -a".

I'm not sure how this could be unit-tested.


Thanks,

Bruce Schuchardt

Re: Review Request 42883: GEODE-871: Frequent use of executeOnAllServers causes build-up of sockets in TIME_WAIT on locator machine

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


> On Jan. 28, 2016, 1:04 a.m., Udo Kohlmeyer wrote:
> > The change in the TCPServer,
> > ```
> > DataSerializer.writeObject(response, output);
> > output.flush()
> > ```
> > seems a little redundant. As the writeObject internally already flushes the stream. Maybe test without the flush. But it cannot hurt to have a flush()

I wonder why DataSerializer.writeObject() is flushing the stream?  I wouldn't think we'd want it to do that.


- Bruce


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


On Jan. 28, 2016, 12:24 a.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42883/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2016, 12:24 a.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-871
>     https://issues.apache.org/jira/browse/GEODE-871
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This change-set alters the client to abort its TCP/IP connection to the locator by enabling SO_LINGER and setting the timeout to zero before it closes the connection.  The Locator closes its connection first, which puts it into TIME_WAIT.  The client then aborts the connection, which cleans up the Locator's TIME_WAIT connection.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java 47f50b342e495a25978d86f717b6140b67687618 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpServer.java f52b9ab3a47c9dc4147a852695b789a2d11b958e 
> 
> Diff: https://reviews.apache.org/r/42883/diff/
> 
> 
> Testing
> -------
> 
> Manual testing using TcpClient to send GetAllServersRequests to the locator thousands of times & counting the number of sockets open on the locator machine with "netstat -a".
> 
> I'm not sure how this could be unit-tested.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 42883: GEODE-871: Frequent use of executeOnAllServers causes build-up of sockets in TIME_WAIT on locator machine

Posted by Udo Kohlmeyer <uk...@gmail.com>.


> On Jan. 28, 2016, 1:04 a.m., Udo Kohlmeyer wrote:
> > The change in the TCPServer,
> > ```
> > DataSerializer.writeObject(response, output);
> > output.flush()
> > ```
> > seems a little redundant. As the writeObject internally already flushes the stream. Maybe test without the flush. But it cannot hurt to have a flush()
> 
> Bruce Schuchardt wrote:
>     I wonder why DataSerializer.writeObject() is flushing the stream?  I wouldn't think we'd want it to do that.

InternalDataSerializer.writeSerializableObject() that flushes the stream. Line: 2369. Might be corner case. But would be good to just get some confirmation.


- Udo


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


On Jan. 28, 2016, 12:24 a.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42883/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2016, 12:24 a.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-871
>     https://issues.apache.org/jira/browse/GEODE-871
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This change-set alters the client to abort its TCP/IP connection to the locator by enabling SO_LINGER and setting the timeout to zero before it closes the connection.  The Locator closes its connection first, which puts it into TIME_WAIT.  The client then aborts the connection, which cleans up the Locator's TIME_WAIT connection.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java 47f50b342e495a25978d86f717b6140b67687618 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpServer.java f52b9ab3a47c9dc4147a852695b789a2d11b958e 
> 
> Diff: https://reviews.apache.org/r/42883/diff/
> 
> 
> Testing
> -------
> 
> Manual testing using TcpClient to send GetAllServersRequests to the locator thousands of times & counting the number of sockets open on the locator machine with "netstat -a".
> 
> I'm not sure how this could be unit-tested.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 42883: GEODE-871: Frequent use of executeOnAllServers causes build-up of sockets in TIME_WAIT on locator machine

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


> On Jan. 28, 2016, 1:04 a.m., Udo Kohlmeyer wrote:
> > The change in the TCPServer,
> > ```
> > DataSerializer.writeObject(response, output);
> > output.flush()
> > ```
> > seems a little redundant. As the writeObject internally already flushes the stream. Maybe test without the flush. But it cannot hurt to have a flush()
> 
> Bruce Schuchardt wrote:
>     I wonder why DataSerializer.writeObject() is flushing the stream?  I wouldn't think we'd want it to do that.
> 
> Udo Kohlmeyer wrote:
>     InternalDataSerializer.writeSerializableObject() that flushes the stream. Line: 2369. Might be corner case. But would be good to just get some confirmation.

I don't see a problem with that method.  It's legitimately flushing the ObjectOutputStream that it creates.  ObjectOutputStream has a buffered stream that it writes to and only drains if the buffer is full.


- Bruce


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


On Jan. 28, 2016, 12:24 a.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42883/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2016, 12:24 a.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-871
>     https://issues.apache.org/jira/browse/GEODE-871
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This change-set alters the client to abort its TCP/IP connection to the locator by enabling SO_LINGER and setting the timeout to zero before it closes the connection.  The Locator closes its connection first, which puts it into TIME_WAIT.  The client then aborts the connection, which cleans up the Locator's TIME_WAIT connection.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java 47f50b342e495a25978d86f717b6140b67687618 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpServer.java f52b9ab3a47c9dc4147a852695b789a2d11b958e 
> 
> Diff: https://reviews.apache.org/r/42883/diff/
> 
> 
> Testing
> -------
> 
> Manual testing using TcpClient to send GetAllServersRequests to the locator thousands of times & counting the number of sockets open on the locator machine with "netstat -a".
> 
> I'm not sure how this could be unit-tested.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 42883: GEODE-871: Frequent use of executeOnAllServers causes build-up of sockets in TIME_WAIT on locator machine

Posted by Udo Kohlmeyer <uk...@gmail.com>.

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


Ship it!




The change in the TCPServer,
```
DataSerializer.writeObject(response, output);
output.flush()
```
seems a little redundant. As the writeObject internally already flushes the stream. Maybe test without the flush. But it cannot hurt to have a flush()

- Udo Kohlmeyer


On Jan. 28, 2016, 12:24 a.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42883/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2016, 12:24 a.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-871
>     https://issues.apache.org/jira/browse/GEODE-871
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This change-set alters the client to abort its TCP/IP connection to the locator by enabling SO_LINGER and setting the timeout to zero before it closes the connection.  The Locator closes its connection first, which puts it into TIME_WAIT.  The client then aborts the connection, which cleans up the Locator's TIME_WAIT connection.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java 47f50b342e495a25978d86f717b6140b67687618 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpServer.java f52b9ab3a47c9dc4147a852695b789a2d11b958e 
> 
> Diff: https://reviews.apache.org/r/42883/diff/
> 
> 
> Testing
> -------
> 
> Manual testing using TcpClient to send GetAllServersRequests to the locator thousands of times & counting the number of sockets open on the locator machine with "netstat -a".
> 
> I'm not sure how this could be unit-tested.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 42883: GEODE-871: Frequent use of executeOnAllServers causes build-up of sockets in TIME_WAIT on locator machine

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

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


Ship it!




Ship It!

- Hitesh Khamesra


On Jan. 28, 2016, 12:24 a.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42883/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2016, 12:24 a.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-871
>     https://issues.apache.org/jira/browse/GEODE-871
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This change-set alters the client to abort its TCP/IP connection to the locator by enabling SO_LINGER and setting the timeout to zero before it closes the connection.  The Locator closes its connection first, which puts it into TIME_WAIT.  The client then aborts the connection, which cleans up the Locator's TIME_WAIT connection.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java 47f50b342e495a25978d86f717b6140b67687618 
>   gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpServer.java f52b9ab3a47c9dc4147a852695b789a2d11b958e 
> 
> Diff: https://reviews.apache.org/r/42883/diff/
> 
> 
> Testing
> -------
> 
> Manual testing using TcpClient to send GetAllServersRequests to the locator thousands of times & counting the number of sockets open on the locator machine with "netstat -a".
> 
> I'm not sure how this could be unit-tested.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>