You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sailesh Mukil (Code Review)" <ge...@cloudera.org> on 2018/02/21 21:15:56 UTC

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Sailesh Mukil has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9383


Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................

KUDU-2301: (Part-1) Add instrumentation on a per connection level

This patch returns the OutboundTransfer queue size on a per
connection level and makes them accessible via the
DumpRunningRpcs() call.

A test is added in rpc-test to ensure that this metric works as
expected.

A future patch will add more metrics.

Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
---
M be/src/kudu/rpc/connection.cc
M be/src/kudu/rpc/connection.h
M be/src/kudu/rpc/messenger.h
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/rpc/rpc_introspection.proto
5 files changed, 77 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/9383/1
-- 
To view, visit http://gerrit.cloudera.org:8080/9383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9383 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................


Patch Set 1:

This patch isn't merged on the Kudu side yet and is still under review. I've pushed it now so that it's easier to review the patch that sits on top of this:
https://gerrit.cloudera.org/#/c/9384/

I'll rebase and update this patch after the review is complete on the Kudu side.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 21:17:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/9383 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................


Patch Set 1: Code-Review+1

Looks good to me, manually compared the two diffs. Can you add some pointer to the commit message to point to the Kudu review? Usually cherry-pick would have added those I think (e.g. https://gerrit.cloudera.org/#/c/9361/).


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 23:17:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9383 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 28 Feb 2018 23:04:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9383 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................

KUDU-2301: (Part-1) Add instrumentation on a per connection level

This patch returns the OutboundTransfer queue size on a per
connection level and makes them accessible via the
DumpRunningRpcs() call.

A test is added in rpc-test to ensure that this metric works as
expected.

A future patch will add more metrics.

Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Reviewed-on: http://gerrit.cloudera.org:8080/9343
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/9383
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Reviewed-by: Michael Ho <kw...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/kudu/rpc/connection.cc
M be/src/kudu/rpc/connection.h
M be/src/kudu/rpc/messenger.h
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/rpc/rpc_introspection.proto
5 files changed, 77 insertions(+), 1 deletion(-)

Approvals:
  Sailesh Mukil: Looks good to me, but someone else must approve
  Michael Ho: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9383 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................


Patch Set 2: Code-Review+1

> Looks good to me, manually compared the two diffs. Can you add some
 > pointer to the commit message to point to the Kudu review? Usually
 > cherry-pick would have added those I think (e.g. https://gerrit.cloudera.org/#/c/9361/).

Done. Carry Lars' +1.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 22 Feb 2018 23:55:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9383 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 26 Feb 2018 23:51:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Lars Volker, Kudu Jenkins, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................

KUDU-2301: (Part-1) Add instrumentation on a per connection level

This patch returns the OutboundTransfer queue size on a per
connection level and makes them accessible via the
DumpRunningRpcs() call.

A test is added in rpc-test to ensure that this metric works as
expected.

A future patch will add more metrics.

Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Reviewed-on: http://gerrit.cloudera.org:8080/9343
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M be/src/kudu/rpc/connection.cc
M be/src/kudu/rpc/connection.h
M be/src/kudu/rpc/messenger.h
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/rpc/rpc_introspection.proto
5 files changed, 77 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/9383/2
-- 
To view, visit http://gerrit.cloudera.org:8080/9383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9383 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................


Patch Set 1:

> This patch isn't merged on the Kudu side yet and is still under
 > review. I've pushed it now so that it's easier to review the patch
 > that sits on top of this:
 > https://gerrit.cloudera.org/#/c/9384/
 > 
 > I'll rebase and update this patch after the review is complete on
 > the Kudu side.

This patch has been merged on the Kudu side now, in this same form. So no rebase with Kudu required.

It is now ready for review.

Cherry-pick had small conflicts for some #includes. Nothing major.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 18:11:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9383 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection level
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2031/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9383
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 28 Feb 2018 19:14:36 +0000
Gerrit-HasComments: No