You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2018/02/28 21:29:51 UTC
[kudu-CR] Make metrics name matching case-insensitive
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9462
Change subject: Make metrics name matching case-insensitive
......................................................................
Make metrics name matching case-insensitive
Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
---
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
2 files changed, 13 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/9462/1
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9462
to look at the new patch set (#2).
Change subject: Make metrics name matching case-insensitive
......................................................................
Make metrics name matching case-insensitive
Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
---
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
2 files changed, 18 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/9462/2
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9462
to look at the new patch set (#3).
Change subject: Make metrics name matching case-insensitive
......................................................................
Make metrics name matching case-insensitive
Some metrics have mixed-case name, for example
handler_latency_kudu_tserver_TabletCopyService_BeginTabletCopySession
Previously, filtered metrics like /metrics?metrics=copy would omit
this metric since "Copy" doesn't match "copy". This fixes it so
the above metric would be returned.
Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
---
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
2 files changed, 18 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/9462/3
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 )
Change subject: Make metrics name matching case-insensitive
......................................................................
Patch Set 1:
Add a test or check that metrics are unique when keyed by all-lowercase name.
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Feb 2018 23:31:04 +0000
Gerrit-HasComments: No
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9462 )
Change subject: Make metrics name matching case-insensitive
......................................................................
Make metrics name matching case-insensitive
Some metrics have mixed-case name, for example
handler_latency_kudu_tserver_TabletCopyService_BeginTabletCopySession
Previously, filtered metrics like /metrics?metrics=copy would omit
this metric since "Copy" doesn't match "copy". This fixes it so
the above metric would be returned.
Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Reviewed-on: http://gerrit.cloudera.org:8080/9462
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
2 files changed, 18 insertions(+), 2 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 )
Change subject: Make metrics name matching case-insensitive
......................................................................
Patch Set 1:
(2 comments)
> Code looks fine but what's the motivation for this change?
Some metrics have mixed case names, e.g. "handler_latency_kudu_tserver_TabletCopyService_BeginTabletCopySession". While working on an issue with Mike recently, we wanted to look at all tablet-copy-related metrics, so we hit /metrics?metrics=copy and were surprised to see the above metric missing.
http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics-test.cc
File src/kudu/util/metrics-test.cc:
http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics-test.cc@214
PS1, Line 214: // Verify that filtering is case-insensitive.
> While you're there, maybe also test that it's indeed a substring match?
Done
http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics.cc@194
PS1, Line 194: string param_uc;
> Nit: could be declared inside the loop.
Done
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Mar 2018 00:12:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: Make metrics name matching case-insensitive
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 )
Change subject: Make metrics name matching case-insensitive
......................................................................
Patch Set 2: Verified+1
> Patch Set 2: Verified-1
>
> Build Failed
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/12283/ : FAILURE
The build failure seems unrelated, but scary. Looking into it.
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Mar 2018 01:04:09 +0000
Gerrit-HasComments: No
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 )
Change subject: Make metrics name matching case-insensitive
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:10:16 +0000
Gerrit-HasComments: No
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 )
Change subject: Make metrics name matching case-insensitive
......................................................................
Patch Set 1:
(2 comments)
Code looks fine but what's the motivation for this change?
http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics-test.cc
File src/kudu/util/metrics-test.cc:
http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics-test.cc@214
PS1, Line 214: // Verify that filtering is case-insensitive.
While you're there, maybe also test that it's indeed a substring match?
http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics.cc@194
PS1, Line 194: string param_uc;
Nit: could be declared inside the loop.
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Mar 2018 19:38:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] Make metrics name matching case-insensitive
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 )
Change subject: Make metrics name matching case-insensitive
......................................................................
Patch Set 2:
> Some metrics have mixed case names, e.g. "handler_latency_kudu_tserver_TabletCopyService_BeginTabletCopySession". While working on an issue with Mike recently, we wanted to look at all tablet-copy-related metrics, so we hit /metrics?metrics=copy and were surprised to see the above metric missing.
Gotcha. Could you include that use case in the commit message, so it's more clear what sort of new functionality is enabled by the commit?
--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Mar 2018 00:19:45 +0000
Gerrit-HasComments: No