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 2017/08/23 22:07:22 UTC

[kudu-CR] Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"

Hello Michael Ho,

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

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

to review the following change.

Change subject: Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"
......................................................................

Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"

This reverts commit 351337ee2d92361e2c87699d1c87a49774a3ab4d.

This patch seemed to cause a problem in test cluster that runs YCSB
stress tests. An RPC client and server got "out of sync" somehow on a
connection so that the footer of one call was interpreted as the length
prefix of the next call by the server. It then expected a 40MB+ RPC and
just sat there consuming RPC calls for many hours assuming it was all
part of one large RPC. The other end just kept timing out since it was
sending calls and not getting any responses.

Michael and I looked into it for a while and found one issue but still
couldn't fully explain how this happened, so we'll revert for now.

Change-Id: I1430cabfc54e858c1ad073574404821ae71c169f
---
M docs/design-docs/rpc.md
M src/kudu/rpc/blocking_ops.cc
M src/kudu/rpc/connection.cc
M src/kudu/rpc/connection.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/inbound_call.h
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/rtest.proto
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/serialization.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/transfer.cc
M src/kudu/rpc/transfer.h
19 files changed, 157 insertions(+), 530 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1430cabfc54e858c1ad073574404821ae71c169f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Michael Ho

[kudu-CR] Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"

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

Change subject: Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"
......................................................................


Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"

This reverts commit 351337ee2d92361e2c87699d1c87a49774a3ab4d.

This patch seemed to cause a problem in test cluster that runs YCSB
stress tests. An RPC client and server got "out of sync" somehow on a
connection so that the footer of one call was interpreted as the length
prefix of the next call by the server. It then expected a 40MB+ RPC and
just sat there consuming RPC calls for many hours assuming it was all
part of one large RPC. The other end just kept timing out since it was
sending calls and not getting any responses.

Michael and I looked into it for a while and found one issue but still
couldn't fully explain how this happened, so we'll revert for now.

Change-Id: I1430cabfc54e858c1ad073574404821ae71c169f
Reviewed-on: http://gerrit.cloudera.org:8080/7788
Tested-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M docs/design-docs/rpc.md
M src/kudu/rpc/blocking_ops.cc
M src/kudu/rpc/connection.cc
M src/kudu/rpc/connection.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/inbound_call.h
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/rtest.proto
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/serialization.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/transfer.cc
M src/kudu/rpc/transfer.h
19 files changed, 157 insertions(+), 530 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1430cabfc54e858c1ad073574404821ae71c169f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"

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

Change subject: Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"
......................................................................


Patch Set 1: Code-Review+2

Going to just +2 this since it's a revert

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1430cabfc54e858c1ad073574404821ae71c169f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"

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

Change subject: Revert "KUDU-2065, KUDU-2011: Release sidecars on cancellation or timeout"
......................................................................


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1430cabfc54e858c1ad073574404821ae71c169f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No