Yifan Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19411 )

Change subject: [tools] add --tables flag to 'local_replica delete'
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/19411/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19411/3//COMMIT_MSG@9
PS3, Line 9: allow deletion
           : multiple tablets by table
> nit: allow deletion of multiple tablets
Done


http://gerrit.cloudera.org:8080/#/c/19411/3//COMMIT_MSG@11
PS3, Line 11: y multipl
> nit: users
Done


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

http://gerrit.cloudera.org:8080/#/c/19411/3/src/kudu/tools/kudu-tool-test.cc@4208
PS3, Line 4208: specified tablet id does not exist");
> If tabletid is specified and should work just like a pattern, it would make
Done


http://gerrit.cloudera.org:8080/#/c/19411/3/src/kudu/tools/kudu-tool-test.cc@4329
PS3, Line 4329:     ASSERT_EQ(kDefaultTableName, 
tablet_replicas[0]->tablet()->metadata()->table_name());
> nit: Also check the left replica's table name is not kTableName?
Done


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

http://gerrit.cloudera.org:8080/#/c/19411/3/src/kudu/tools/tool_action_local_replica.cc@801
PS3, Line 801:   if (tablet_ids_str.find('*') == string::npos && uniq_count > 
tablets_to_delete.size()) {
> Because the input is pattern now, one pattern may match multiple tablet ids
Yes. So with this change, we can only identify wrong tablet ids but not wrong 
tablet id patterns.


http://gerrit.cloudera.org:8080/#/c/19411/3/src/kudu/tools/tool_action_local_replica.cc@1392
PS3, Line 1392: AddOptionalParameter("tables")
> I am not sure if I get this. Where are we storing table name from user in c
It is stored in gflag value.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d862a715f05179b2e8def0d1cdfe58c32299329
Gerrit-Change-Number: 19411
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <chinazhangyi...@163.com>
Gerrit-Reviewer: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ara...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <chinazhangyi...@163.com>
Gerrit-Reviewer: Yingchun Lai <acelyc1112...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Jan 2023 07:07:33 +0000
Gerrit-HasComments: Yes

Reply via email to