Wang Xixu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19501 )

Change subject: [Tool] Show the information of a tablet
......................................................................


Patch Set 7:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/19501/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19501/5//COMMIT_MSG@9
PS5, Line 9: This tool is used to show the table name, table id and all
           : replicas on different tablet servers according to the tablet
           : id.
> Could give s sample output?
Done


http://gerrit.cloudera.org:8080/#/c/19501/6/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

http://gerrit.cloudera.org:8080/#/c/19501/6/src/kudu/client/client-internal.cc@298
PS6, Line 298: Status KuduClient::Data::GetTablet(KuduClient* client,
> warning: method 'GetTablet' can be made const [readability-make-member-func
Done


http://gerrit.cloudera.org:8080/#/c/19501/6/src/kudu/client/client-internal.cc@373
PS6, Line 373: Status KuduClient::Data::GetTabletServer(KuduClient* client,
> warning: method 'GetTabletServer' can be made const [readability-make-membe
Done


http://gerrit.cloudera.org:8080/#/c/19501/5/src/kudu/master/master.proto
File src/kudu/master/master.proto:

http://gerrit.cloudera.org:8080/#/c/19501/5/src/kudu/master/master.proto@460
PS5, Line 460: es id = 1;
> I guess this field rather belongs to GetTabletLocationsRequestPB?
You are right. I make a mismake.


http://gerrit.cloudera.org:8080/#/c/19501/5/src/kudu/master/master.proto@461
PS5, Line 461:     optional string name = 2;
             :   }
> It would be great to create a dedicated message for this and add it just as
Done


http://gerrit.cloudera.org:8080/#/c/19501/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19501/6/src/kudu/tools/kudu-tool-test.cc@3967
PS6, Line 3967:   ASSERT_STR_CONTAINS(stdout, workload_table->id());
> warning: 'opts' used after it was moved [bugprone-use-after-move]
Done


http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/tools/tool_action_tablet.cc@409
PS3, Line 409: { kTablet
> +1
Done


http://gerrit.cloudera.org:8080/#/c/19501/5/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/19501/5/src/kudu/tools/tool_action_tablet.cc@101
PS5, Line 101:  ShowTabletI
> This is C++, not Java, so I don't understand why to introduce otherwise use
OK. I will use friend function.

That is funny. Shower is an equipment of bathing. So learn English is not only 
about grammar but also culture.

Thank you for your comments.


http://gerrit.cloudera.org:8080/#/c/19501/5/src/kudu/tools/tool_action_tablet.cc@126
PS5, Line 126:   DataTable cmatrix({ "UUID", "Host
> Why would you need this?  std::cout is std::ostream as well, so it fits whe
Done


http://gerrit.cloudera.org:8080/#/c/19501/5/src/kudu/tools/tool_action_tablet.cc@127
PS5, Line 127: r (
> Would it be enough using 'cout' here?
Done


http://gerrit.cloudera.org:8080/#/c/19501/5/src/kudu/tools/tool_action_tablet.cc@136
PS5, Line 136: na
> nit: wrong indent?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5ae5f61f50a44c4787843df76adaa61700ae9fe
Gerrit-Change-Number: 19501
Gerrit-PatchSet: 7
Gerrit-Owner: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: KeDeng <kdeng...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org>
Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com>
Gerrit-Comment-Date: Tue, 14 Mar 2023 13:23:58 +0000
Gerrit-HasComments: Yes

Reply via email to