You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org> on 2021/06/21 15:52:32 UTC
[kudu-CR] [metrics] KUDU-3269: Add UUID of the server into the metrics output
Abhishek Chennaka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17607
Change subject: [metrics] KUDU-3269: Add UUID of the server into the metrics output
......................................................................
[metrics] KUDU-3269: Add UUID of the server into the metrics output
This patch adds the UUID and the server hostname into the metrics
webpage of master and tablet server. These are added in the attributes
section of the metrics webpage. For example:
"type": "server",
"id": "kudu.master",
"attributes": {
"hostname": "MacBook-Pro.local",
"uuid": "4b8015e347dd4efd92bb0d38e03434e6"
},
Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
---
M src/kudu/master/master-test.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 33 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/17607/1
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 5: Verified+1
unrelated test failures in one of Java tests and LogRollingITest.TestLogCleanupOnStartup
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 24 Jun 2021 16:54:34 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/server/server_base.cc@855
PS3, Line 855: GetFQDN
> Is it possible to reuse the result of the prior call to GetFQDN() instead o
The implementation of GetFQDN() involves calling GetHostname() to update the hostname and then re-updating the hostname with FQDN if GetAddrInfo() succeeds, so even if GetFQDN() fails and GetHostname() succeeds, the hostname is updated. So calling GetHostname() again might be redundant
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/tserver/tablet_server-test.cc@3828
PS3, Line 3828: GetHostname
> Is this still working as expected once switched to FQDN?
This should have been GetFQDN(). Will update it.
That was working because in mac we do not update the hostname with FQDN but just with local hostname if there is no domain name as implemented in GetFQDN().
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 24 Jun 2021 02:59:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 24 Jun 2021 16:54:40 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG@16
PS2, Line 16: "hostname": "MacBook-Pro.local",
> I filed KUDU-3269 with the rationale to add identification for collected ta
makes sense, lets keep it simple
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 23 Jun 2021 21:53:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/server/server_base.cc@855
PS3, Line 855: GetFQDN
Is it possible to reuse the result of the prior call to GetFQDN() instead of calling it here once more?
What about falling back to GetHostname() if GetFQDN() didn't succeed? If it's not worth making a call to GetHostName() when GetFQDN() failed, maybe add a small in-line comment for the reasoning behind always sticking to FQDN()?
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/tserver/tablet_server-test.cc@3824
PS3, Line 3824:
nit: this indent seems a bit off
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/tserver/tablet_server-test.cc@3828
PS3, Line 3828: GetHostname
Is this still working as expected once switched to FQDN?
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 23 Jun 2021 20:59:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID of the server into the metrics output
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc@1386
PS2, Line 1386: //Validate the hostname and the UUID of the server from the metrics webpage
nit: add a space before 'V'
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc@1388
PS2, Line 1388:
nit: indent should be 2 spaces
If you are using vim, adding the following into .vimrc should help:
set expandtab
set shiftwidth=2
set tabstop=2
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc@852
PS2, Line 852: if (GetHostname(&server_hostname).ok()) {
I guess it's worth trying to fetch FQDN?
string hostname;
if (!GetFQDN(&hostname).ok()) {
WARN_NOT_OK(GetHostname(&hostname), "could not determine hostname");
}
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc@855
PS2, Line 855: metric_entity_->SetAttribute("hostname", "Could not be determined" );
> I think I would prefer an empty string given the presence of a value could
+1
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc@3820
PS2, Line 3820:
nit: the indent should be 2 spaces
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 23 Jun 2021 14:27:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG@16
PS2, Line 16: "hostname": "MacBook-Pro.local",
> Had a chat with Alexey on the IP address adding part. I think hostname and
I filed KUDU-3269 with the rationale to add identification for collected tablet server metrics, so it's possible to tell which server the metrics were collected from given just the output from the `/metrics` endpoint. UUID and name of the node is enough in that scope.
As for the IP addresses, there are too many options, actually:
* RPC server bound IP address(es)
* Web server bound IP address(es)
* advertised IP address(es)
If adding those IP addresses, I'd vote to do so in a separate patch.
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 23 Jun 2021 20:53:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID of the server into the metrics output
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17607
to look at the new patch set (#2).
Change subject: [metrics] KUDU-3269: Add UUID of the server into the metrics output
......................................................................
[metrics] KUDU-3269: Add UUID of the server into the metrics output
This patch adds the UUID and the server hostname into the metrics
webpage of master and tablet server. These are added in the attributes
section of the metrics webpage. For example:
"type": "server",
"id": "kudu.master",
"attributes": {
"hostname": "MacBook-Pro.local",
"uuid": "4b8015e347dd4efd92bb0d38e03434e6"
},
Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
---
M src/kudu/master/master-test.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 35 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/17607/2
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [metrics] KUDU-3269: Add UUID of the server into the metrics output
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID of the server into the metrics output
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG@16
PS2, Line 16: "hostname": "MacBook-Pro.local",
Can you update this message to indicate that you also added the hostname?
Is it worth adding the IP too?
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc@1387
PS2, Line 1387: ){
Add a space before the brace
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc@1396
PS2, Line 1396: ASSERT_TRUE(GetHostname(&server_hostname).ok());
ASSERT_OK(GetHostname(&server_hostname));
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc@854
PS2, Line 854: e{
Add a space before brace
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc@855
PS2, Line 855: metric_entity_->SetAttribute("hostname", "Could not be determined" );
I think I would prefer an empty string given the presence of a value could be interpreted to be the hostname.
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc@3819
PS2, Line 3819: ){
Add a space before braces
http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc@3828
PS2, Line 3828: ASSERT_TRUE(GetHostname(&server_hostname).ok());
ASSERT_OK(GetHostname(&server_hostname));
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 22 Jun 2021 13:49:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17607
to look at the new patch set (#4).
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
[metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
This patch adds the UUID and the server hostname into the metrics
webpage of master and tablet server. These are added in the attributes
section of the metrics webpage. For example:
"type": "server",
"id": "kudu.master",
"attributes": {
"hostname": "MacBook-Pro.local",
"uuid": "4b8015e347dd4efd92bb0d38e03434e6"
},
Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
---
M src/kudu/master/master-test.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 36 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/17607/4
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
[metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
This patch adds the UUID and the server hostname into the metrics
webpage of master and tablet server. These are added in the attributes
section of the metrics webpage. For example:
"type": "server",
"id": "kudu.master",
"attributes": {
"hostname": "MacBook-Pro.local",
"uuid": "4b8015e347dd4efd92bb0d38e03434e6"
},
Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Reviewed-on: http://gerrit.cloudera.org:8080/17607
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/master/master-test.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 35 insertions(+), 0 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17607
to look at the new patch set (#3).
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
[metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
This patch adds the UUID and the server hostname into the metrics
webpage of master and tablet server. These are added in the attributes
section of the metrics webpage. For example:
"type": "server",
"id": "kudu.master",
"attributes": {
"hostname": "MacBook-Pro.local",
"uuid": "4b8015e347dd4efd92bb0d38e03434e6"
},
Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
---
M src/kudu/master/master-test.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 34 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/17607/3
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/17607/3/src/kudu/server/server_base.cc@855
PS3, Line 855:
> The implementation of GetFQDN() involves calling GetHostname() to update th
Ah, that makes sense. For some reason, I thought we need to tell between failures of GetHostname() and GetAddrInfo() in the implementation of GetFQDN().
It seems in this case supplying either an empty string or the result of GetHostname() is OK, and the same warning would be generated in both cases. Which is not perfect, but it's OK in this context.
Overall, looks acceptable. Thank you for adding the comment.
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 24 Jun 2021 06:31:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 4: Code-Review+1
It seems LINT and IWYU are not yet happy. I'd try running 'make ilint' and 'make iwyu' and correct the reported issues.
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 24 Jun 2021 06:32:43 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] KUDU-3269: Add UUID of the server into the metrics output
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID of the server into the metrics output
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17607/1/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/17607/1/src/kudu/server/server_base.cc@852
PS1, Line 852: if (GetHostname(&server_hostname).ok()) {
: metric_entity_->SetAttribute("hostname", server_hostname );
: }
Nit: In case of error, instead of omitting the attribute entirely should the value be empty/null?
One minor benefit is that the caller doesn't have to check explicitly for the presence of the hostname key if uuid is present.
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 21 Jun 2021 16:01:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID of the server into the metrics output
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID of the server into the metrics output
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG@16
PS2, Line 16: "hostname": "MacBook-Pro.local",
> Can you update this message to indicate that you also added the hostname?
Had a chat with Alexey on the IP address adding part. I think hostname and UUID should be good enough to identify the host from metrics JSON output
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 23 Jun 2021 19:06:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17607/4/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/17607/4/src/kudu/server/server_base.cc@94
PS4, Line 94: #include "kudu/util/net/net_util.h"
This is already included at line 82 above
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 24 Jun 2021 14:01:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17607
to look at the new patch set (#5).
Change subject: [metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
......................................................................
[metrics] KUDU-3269: Add UUID and hostname of the server into the metrics output
This patch adds the UUID and the server hostname into the metrics
webpage of master and tablet server. These are added in the attributes
section of the metrics webpage. For example:
"type": "server",
"id": "kudu.master",
"attributes": {
"hostname": "MacBook-Pro.local",
"uuid": "4b8015e347dd4efd92bb0d38e03434e6"
},
Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
---
M src/kudu/master/master-test.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/17607/5
--
To view, visit http://gerrit.cloudera.org:8080/17607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)