You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dinesh Bhat (Code Review)" <ge...@cloudera.org> on 2016/11/04 22:08:44 UTC
[kudu-CR] [rpc] Close socket on non-linux platforms during RPC shutdown
Hello Mike Percy, Adar Dembo, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4958
to review the following change.
Change subject: [rpc] Close socket on non-linux platforms during RPC shutdown
......................................................................
[rpc] Close socket on non-linux platforms during RPC shutdown
While using a MiniTabletServer::Restart() functionality, it was observed
that tablet server was not able to bind to same socket again on OS X.
It turns out non-linux platforms can leave the stale sockets behind
when we shutdown the RPC server. While shutdown() is not available on
platforms other than linux, we should be able to close() the socket on
other platforms to make sure stale fds are not left behind.
NOTE: This change is expected to impact non-linux platforms only.
Traces from the log which originally observed the issue:
W1104 14:58:36.040030 1953316864 net_util.cc:293] Failed to bind to 0.0.0.0:56021. Trying to use lsof to find any processes listening on the same port:
I1104 14:58:36.040457 1953316864 net_util.cc:296] $ lsof -n -i 'TCP:56021' -sTCP:LISTEN ; for pid in $(lsof -F p -n -i 'TCP:56021' -sTCP:LISTEN | cut -f 2 -dp) ; do pstree $pid || ps h -p $pid;done
W1104 14:58:36.147557 1953316864 net_util.cc:303] COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
rpc-test 38600 dinesh 8u IPv4 0xcf92530d59e2332d 0t0 TCP *:56021 (LISTEN)
-+= 38600 dinesh ./bin/rpc-test --gtest_filter=TestRpc.TestAcceptorPoolSocketShutDown
\-+- 38601 dinesh bash -c lsof -n -i 'TCP:56021' -sTCP:LISTEN ; for pid in $(lsof -F p -n -i 'TCP:56021' -sTCP:LISTEN | cut -f 2 -dp) ; do pstree $pid || ps h -p $pid;done
\-+- 38608 dinesh pstree 38600
\--- 38609 root ps -axwwo user,pid,ppid,pgid,command
../../src/kudu/rpc/rpc-test.cc:106: Failure
Failed
Bad status: Network error: error binding socket to 0.0.0.0:56021: Address already in use (error 48)
Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
---
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/rpc-test.cc
2 files changed, 21 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/4958/1
--
To view, visit http://gerrit.cloudera.org:8080/4958
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [rpc] Close socket on non-linux platforms during RPC shutdown
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: [rpc] Close socket on non-linux platforms during RPC shutdown
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
lgtm
http://gerrit.cloudera.org:8080/#/c/4958/1/src/kudu/rpc/acceptor_pool.cc
File src/kudu/rpc/acceptor_pool.cc:
Line 103: // Calling shutdown on an accepting (non-connected) socket is illegal on most
Update comment
http://gerrit.cloudera.org:8080/#/c/4958/1/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
Line 95: TEST_F(TestRpc, TestAcceptorPoolSocketShutDown) {
nit: s/ShutDown/Shutdown/ for consistency w/ rest of the code base, here and below
--
To view, visit http://gerrit.cloudera.org:8080/4958
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [rpc] Close socket on non-linux platforms during RPC shutdown
Posted by "Dinesh Bhat (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Mike Percy, Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4958
to look at the new patch set (#2).
Change subject: [rpc] Close socket on non-linux platforms during RPC shutdown
......................................................................
[rpc] Close socket on non-linux platforms during RPC shutdown
While using a MiniTabletServer::Restart() functionality, it was observed
that tablet server was not able to bind to same socket again on OS X.
It turns out non-linux platforms can leave the stale sockets behind
when we shutdown the RPC server. While shutdown() is not available on
platforms other than linux, we should be able to close() the socket on
other platforms to make sure stale fds are not left behind.
NOTE: This change is expected to impact non-linux platforms only.
Traces from the log which originally observed the issue:
W1104 14:58:36.040030 1953316864 net_util.cc:293] Failed to bind to 0.0.0.0:56021. Trying to use lsof to find any processes listening on the same port:
I1104 14:58:36.040457 1953316864 net_util.cc:296] $ lsof -n -i 'TCP:56021' -sTCP:LISTEN ; for pid in $(lsof -F p -n -i 'TCP:56021' -sTCP:LISTEN | cut -f 2 -dp) ; do pstree $pid || ps h -p $pid;done
W1104 14:58:36.147557 1953316864 net_util.cc:303] COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
rpc-test 38600 dinesh 8u IPv4 0xcf92530d59e2332d 0t0 TCP *:56021 (LISTEN)
-+= 38600 dinesh ./bin/rpc-test --gtest_filter=TestRpc.TestAcceptorPoolSocketShutDown
\-+- 38601 dinesh bash -c lsof -n -i 'TCP:56021' -sTCP:LISTEN ; for pid in $(lsof -F p -n -i 'TCP:56021' -sTCP:LISTEN | cut -f 2 -dp) ; do pstree $pid || ps h -p $pid;done
\-+- 38608 dinesh pstree 38600
\--- 38609 root ps -axwwo user,pid,ppid,pgid,command
../../src/kudu/rpc/rpc-test.cc:106: Failure
Failed
Bad status: Network error: error binding socket to 0.0.0.0:56021: Address already in use (error 48)
Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
---
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/rpc-test.cc
M src/kudu/util/net/socket.cc
3 files changed, 29 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/4958/2
--
To view, visit http://gerrit.cloudera.org:8080/4958
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [rpc] Close socket on non-linux platforms during RPC shutdown
Posted by "Dinesh Bhat (Code Review)" <ge...@cloudera.org>.
Dinesh Bhat has posted comments on this change.
Change subject: [rpc] Close socket on non-linux platforms during RPC shutdown
......................................................................
Patch Set 2:
(2 comments)
TFTR Mike.
http://gerrit.cloudera.org:8080/#/c/4958/1/src/kudu/rpc/acceptor_pool.cc
File src/kudu/rpc/acceptor_pool.cc:
Line 103: // Calling shutdown on an accepting (non-connected) socket is illegal on most
> Update comment
Done
http://gerrit.cloudera.org:8080/#/c/4958/1/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
Line 95: // the first pool has been shutdown.
> nit: s/ShutDown/Shutdown/ for consistency w/ rest of the code base, here an
Done
--
To view, visit http://gerrit.cloudera.org:8080/4958
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [rpc] Close socket on non-linux platforms during RPC shutdown
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: [rpc] Close socket on non-linux platforms during RPC shutdown
......................................................................
Patch Set 2:
Do we still think this is a useful change?
--
To view, visit http://gerrit.cloudera.org:8080/4958
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [rpc] Close socket on non-linux platforms during RPC shutdown
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has abandoned this change.
Change subject: [rpc] Close socket on non-linux platforms during RPC shutdown
......................................................................
Abandoned
The issue has been fixed in another changelist (in the new approach the socket is closed after shutdown regardless of the platform): https://gerrit.cloudera.org/#/c/6665
--
To view, visit http://gerrit.cloudera.org:8080/4958
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <di...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>