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/06/01 22:07:26 UTC

[kudu-CR] [tools] Add version info and check to ksck

Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10579


Change subject: [tools] Add version info and check to ksck
......................................................................

[tools] Add version info and check to ksck

This adds a new version summary table, similar to the one on the
master web ui for tablet servers, that summarizes the versions of Kudu
running in the cluster (both masters and tablet servers). ksck will
issue a warning if there are multiple versions. The version info is
available as part of the server health summaries in the KsckResults, so
clients of the Ksck class can use this information. Version info is
plumbed into the JSON output as well.

An example table and warning:

Version Summary
    Version     |                                                  Server
----------------+-----------------------------------------------------------------------------------------------------
 1.6.0          | master@master-0.foo.com, tserver@tserver-0.foo.com, tserver@tserver-1.foo.com, and 1 other server(s)
 1.7.0          | master@master-1.foo.com
 1.8.0-SNAPSHOT | master@master-2.foo.com

==================
Warnings:
==================
version check error: not all servers are running the same version: 3 different versions were seen

Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
M src/kudu/tools/tool.proto
7 files changed, 116 insertions(+), 3 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add version info and check to ksck

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

Change subject: [tools] Add version info and check to ksck
......................................................................


Patch Set 1:

(5 comments)

Overall looks good, just a couple of nits.

http://gerrit.cloudera.org:8080/#/c/10579/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10579/1//COMMIT_MSG@20
PS1, Line 20: Server
nit: Servers ?


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:

http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h@246
PS1, Line 246: boost::optional<std::string>
why not a const reference?


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h@371
PS1, Line 371: boost::optional<std::string>
maybe, return a const reference?


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_remote.cc@199
PS1, Line 199: version_ = resp.status().version_info().version_string();
Maybe, it makes sense to populate version_ field for a server with WRONG_SERVER_UUID health status?


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:

http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_results.cc@407
PS1, Line 407: Server
nit: Servers or Server(s) ?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Sat, 02 Jun 2018 00:00:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] Add version info and check to ksck

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: [tools] Add version info and check to ksck
......................................................................

[tools] Add version info and check to ksck

This adds a new version summary table, similar to the one on the
master web ui for tablet servers, that summarizes the versions of Kudu
running in the cluster (both masters and tablet servers). ksck will
issue a warning if there are multiple versions. The version info is
available as part of the server health summaries in the KsckResults, so
clients of the Ksck class can use this information. Version info is
plumbed into the JSON output as well.

An example table and warning:

Version Summary
    Version     |                                                  Servers
----------------+-----------------------------------------------------------------------------------------------------
 1.6.0          | master@master-0.foo.com, tserver@tserver-0.foo.com, tserver@tserver-1.foo.com, and 1 other server(s)
 1.7.0          | master@master-1.foo.com
 1.8.0-SNAPSHOT | master@master-2.foo.com

==================
Warnings:
==================
version check error: not all servers are running the same version: 3 different versions were seen

Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
M src/kudu/tools/tool.proto
7 files changed, 118 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/10579/4
-- 
To view, visit http://gerrit.cloudera.org:8080/10579
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add version info and check to ksck

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: [tools] Add version info and check to ksck
......................................................................

[tools] Add version info and check to ksck

This adds a new version summary table, similar to the one on the
master web ui for tablet servers, that summarizes the versions of Kudu
running in the cluster (both masters and tablet servers). ksck will
issue a warning if there are multiple versions. The version info is
available as part of the server health summaries in the KsckResults, so
clients of the Ksck class can use this information. Version info is
plumbed into the JSON output as well.

An example table and warning:

Version Summary
    Version     |                                                  Servers
----------------+-----------------------------------------------------------------------------------------------------
 1.6.0          | master@master-0.foo.com, tserver@tserver-0.foo.com, tserver@tserver-1.foo.com, and 1 other server(s)
 1.7.0          | master@master-1.foo.com
 1.8.0-SNAPSHOT | master@master-2.foo.com

==================
Warnings:
==================
version check error: not all servers are running the same version: 3 different versions were seen

Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
M src/kudu/tools/tool.proto
7 files changed, 117 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/10579/2
-- 
To view, visit http://gerrit.cloudera.org:8080/10579
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] [tools] Add version info and check to ksck

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

Change subject: [tools] Add version info and check to ksck
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/10579/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10579/1//COMMIT_MSG@20
PS1, Line 20: Server
> nit: Servers ?
Done


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:

http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h@246
PS1, Line 246: boost::optional<std::string>
> why not a const reference?
Done


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h@371
PS1, Line 371: boost::optional<std::string>
> maybe, return a const reference?
Done


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:

http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.cc@90
PS1, Line 90: using std::map;
> warning: using decl 'map' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.cc@94
PS1, Line 94: using std::setw;
> warning: using decl 'setw' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_remote.cc@199
PS1, Line 199: version_ = resp.status().version_info().version_string();
> Maybe, it makes sense to populate version_ field for a server with WRONG_SE
Done


http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:

http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_results.cc@407
PS1, Line 407: Server
> nit: Servers or Server(s) ?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 04 Jun 2018 14:58:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] Add version info and check to ksck

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10579 )

Change subject: [tools] Add version info and check to ksck
......................................................................

[tools] Add version info and check to ksck

This adds a new version summary table, similar to the one on the
master web ui for tablet servers, that summarizes the versions of Kudu
running in the cluster (both masters and tablet servers). ksck will
issue a warning if there are multiple versions. The version info is
available as part of the server health summaries in the KsckResults, so
clients of the Ksck class can use this information. Version info is
plumbed into the JSON output as well.

An example table and warning:

Version Summary
    Version     |                                                  Servers
----------------+-----------------------------------------------------------------------------------------------------
 1.6.0          | master@master-0.foo.com, tserver@tserver-0.foo.com, tserver@tserver-1.foo.com, and 1 other server(s)
 1.7.0          | master@master-1.foo.com
 1.8.0-SNAPSHOT | master@master-2.foo.com

==================
Warnings:
==================
version check error: not all servers are running the same version: 3 different versions were seen

Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Reviewed-on: http://gerrit.cloudera.org:8080/10579
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
M src/kudu/tools/tool.proto
7 files changed, 118 insertions(+), 9 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add version info and check to ksck

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

Change subject: [tools] Add version info and check to ksck
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 05 Jun 2018 00:03:23 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add version info and check to ksck

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

Change subject: [tools] Add version info and check to ksck
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10579/2/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:

http://gerrit.cloudera.org:8080/#/c/10579/2/src/kudu/tools/ksck.cc@89
PS2, Line 89: using std::left;
> warning: using decl 'left' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/10579/2/src/kudu/tools/ksck.cc@95
PS2, Line 95: using std::to_string;
> warning: using decl 'to_string' is unused [misc-unused-using-decls]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 04 Jun 2018 15:38:04 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] Add version info and check to ksck

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: [tools] Add version info and check to ksck
......................................................................

[tools] Add version info and check to ksck

This adds a new version summary table, similar to the one on the
master web ui for tablet servers, that summarizes the versions of Kudu
running in the cluster (both masters and tablet servers). ksck will
issue a warning if there are multiple versions. The version info is
available as part of the server health summaries in the KsckResults, so
clients of the Ksck class can use this information. Version info is
plumbed into the JSON output as well.

An example table and warning:

Version Summary
    Version     |                                                  Servers
----------------+-----------------------------------------------------------------------------------------------------
 1.6.0          | master@master-0.foo.com, tserver@tserver-0.foo.com, tserver@tserver-1.foo.com, and 1 other server(s)
 1.7.0          | master@master-1.foo.com
 1.8.0-SNAPSHOT | master@master-2.foo.com

==================
Warnings:
==================
version check error: not all servers are running the same version: 3 different versions were seen

Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
M src/kudu/tools/tool.proto
7 files changed, 118 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/10579/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10579
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50
Gerrit-Change-Number: 10579
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>