You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/04/18 07:40:15 UTC

[kudu-CR] [rpc] close socket on AcceptorPool::Shutdown()

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/6665

Change subject: [rpc] close socket on AcceptorPool::Shutdown()
......................................................................

[rpc] close socket on AcceptorPool::Shutdown()

Close the underlying socket upon AcceptorPool::Shutdown() call. It does
not make much sense keeping the socket open after calling shutdown()
for both send and receive.

Due to the typical ownership pattern of AcceptorPool two references to
the object is kept: one reference is kept by RpcService, another by
Messenger. So, to close the socket it's necessary to call both
{RpcService,Messenger}::Shutdown() methods.

This patch also fixes delete_tablet-itest on MacOS X.

Change-Id: Ided3643b07e642beb586e51efa442b39b3785916
---
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/messenger.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_server.h
M src/kudu/util/net/socket.cc
6 files changed, 31 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/6665/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ided3643b07e642beb586e51efa442b39b3785916
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [rpc] close socket on AcceptorPool::Shutdown()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [rpc] close socket on AcceptorPool::Shutdown()
......................................................................


Patch Set 2:

> Looks like Dinesh did the same thing back in https://gerrit.cloudera.org/#/c/4958.
 > I wonder why that patch was never merged.

Oops, I missed that one, sorry.  Probably I should have updated it instead of creating a new one.

BTW, that patch did close only for MacOS X, not for Linux.  This does so regardless of platform.

-- 
To view, visit http://gerrit.cloudera.org:8080/6665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ided3643b07e642beb586e51efa442b39b3785916
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [rpc] close socket on AcceptorPool::Shutdown()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.

Change subject: [rpc] close socket on AcceptorPool::Shutdown()
......................................................................


[rpc] close socket on AcceptorPool::Shutdown()

Close the underlying socket upon AcceptorPool::Shutdown() call. It does
not make much sense keeping the socket open after calling shutdown()
for both send and receive.

Due to the typical ownership pattern of AcceptorPool two references to
the object is kept: one reference is kept by RpcService, another by
Messenger. So, to close the socket it's necessary to call both
{RpcService,Messenger}::Shutdown() methods.

This patch also fixes delete_tablet-itest on MacOS X.

Change-Id: Ided3643b07e642beb586e51efa442b39b3785916
Reviewed-on: http://gerrit.cloudera.org:8080/6665
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/messenger.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_server.h
M src/kudu/util/net/socket.cc
6 files changed, 31 insertions(+), 26 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/6665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ided3643b07e642beb586e51efa442b39b3785916
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [rpc] close socket on AcceptorPool::Shutdown()

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: [rpc] close socket on AcceptorPool::Shutdown()
......................................................................


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/6665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ided3643b07e642beb586e51efa442b39b3785916
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [rpc] close socket on AcceptorPool::Shutdown()

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: [rpc] close socket on AcceptorPool::Shutdown()
......................................................................


Patch Set 2:

Looks like Dinesh did the same thing back in https://gerrit.cloudera.org/#/c/4958. I wonder why that patch was never merged.

-- 
To view, visit http://gerrit.cloudera.org:8080/6665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ided3643b07e642beb586e51efa442b39b3785916
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No