You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/12/06 04:38:14 UTC

[kudu-CR] rpc: show outbound call state in /rpcz dump

Hello Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: rpc: show outbound call state in /rpcz dump
......................................................................

rpc: show outbound call state in /rpcz dump

Recently I've been looking at a stress cluster that is exhibiting lots
of consensus request timeouts (eg KUDU-1788). It seems that many of the
requests are timing out while the call is still in the process of being
transferred, or in some cases not even sent yet. However, that wasn't
obvious and took a lot of spelunking to figure out what was going on.

This adds a new state to the OutboundCall state machine for 'SENDING',
which is entered when we first start sending the request over the
socket. As before, it transitions to 'SENT' once the request has been
completely transferred.

The state at the time of the timeout is now also put into the
Status::TimedOut error string. It's a little bit ugly, but should be
very useful to see when an issue is client-side or network-related
rather than a server-side call timeout.

Change-Id: Id52bc627a25be87a73b4b75941d7dcc2cf95eaba
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/rpc_introspection.proto
4 files changed, 71 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id52bc627a25be87a73b4b75941d7dcc2cf95eaba
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] rpc: show outbound call state in /rpcz dump

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

Change subject: rpc: show outbound call state in /rpcz dump
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5371/1/src/kudu/rpc/rpc_introspection.proto
File src/kudu/rpc/rpc_introspection.proto:

Line 32:   enum State {
Why not completely replace the State enum in outbound_call.h with this one?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id52bc627a25be87a73b4b75941d7dcc2cf95eaba
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] rpc: show outbound call state in /rpcz dump

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

Change subject: rpc: show outbound call state in /rpcz dump
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5371/1/src/kudu/rpc/rpc_introspection.proto
File src/kudu/rpc/rpc_introspection.proto:

Line 32:   enum State {
> Why not completely replace the State enum in outbound_call.h with this one?
that would work at the moment, but I'm planning to also add states for InboundCall, and I'd prefer the implementation enums to not have "irrelevant" values leaking from inbound to outbound and vice versa.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id52bc627a25be87a73b4b75941d7dcc2cf95eaba
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@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: show outbound call state in /rpcz dump

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

Change subject: rpc: show outbound call state in /rpcz dump
......................................................................


rpc: show outbound call state in /rpcz dump

Recently I've been looking at a stress cluster that is exhibiting lots
of consensus request timeouts (eg KUDU-1788). It seems that many of the
requests are timing out while the call is still in the process of being
transferred, or in some cases not even sent yet. However, that wasn't
obvious and took a lot of spelunking to figure out what was going on.

This adds a new state to the OutboundCall state machine for 'SENDING',
which is entered when we first start sending the request over the
socket. As before, it transitions to 'SENT' once the request has been
completely transferred.

The state at the time of the timeout is now also put into the
Status::TimedOut error string. It's a little bit ugly, but should be
very useful to see when an issue is client-side or network-related
rather than a server-side call timeout.

Change-Id: Id52bc627a25be87a73b4b75941d7dcc2cf95eaba
Reviewed-on: http://gerrit.cloudera.org:8080/5371
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/rpc_introspection.proto
4 files changed, 71 insertions(+), 9 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id52bc627a25be87a73b4b75941d7dcc2cf95eaba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>