Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10536 )

Change subject: [kudu CLI] kudu tablet set_attributes sub-command
......................................................................


Patch Set 1:

(6 comments)

What happens if you mark a tablet REPLACE or PROMOTE in an otherwise stable 
tablet? How should this new tool be tested?

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

http://gerrit.cloudera.org:8080/#/c/10536/1//COMMIT_MSG@7
PS1, Line 7: kudu CLI
I usually use [tools] for this.


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

http://gerrit.cloudera.org:8080/#/c/10536/1/src/kudu/tools/tool_action_tablet.cc@599
PS1, Line 599: *change->mutable_peer()->mutable_permanent_uuid()
Can use set_permanent_uuid().


http://gerrit.cloudera.org:8080/#/c/10536/1/src/kudu/tools/tool_action_tablet.cc@600
PS1, Line 600: 
change->mutable_peer()->mutable_attrs()->set_promote(FLAGS_attribute_promote);
             :   
change->mutable_peer()->mutable_attrs()->set_replace(FLAGS_attribute_replace);
Does it make sense to set both? What would happen? Should the tool reject that, 
or can we rely on the leader to do that?


http://gerrit.cloudera.org:8080/#/c/10536/1/src/kudu/tools/tool_action_tablet.cc@606
PS1, Line 606: RETURN_NOT_OK(proxy->BulkChangeConfig(req, &resp, &rpc));
             :   if (resp.has_error()) {
             :     // The status of the request might be InvalidArgument() if 
the
             :     // replica already has the attributes set as specified in 
the incoming
             :     // request.
             :     return StatusFromPB(resp.error().status());
             :   }
             :
             :   return Status::OK();
aside: I feel like I see this pattern a lot. I wonder if it'd be worth a macro.


http://gerrit.cloudera.org:8080/#/c/10536/1/src/kudu/tools/tool_action_tablet.cc@675
PS1, Line 675: /reset
We can only set.


http://gerrit.cloudera.org:8080/#/c/10536/1/src/kudu/tools/tool_action_tablet.cc@676
PS1, Line 676: replica
The BulkConfigChange is replicated, right? So it's the tablet's configuration. 
I think you mean "Use this sub-command to set a replica's attribute in its 
tablet's Raft configuration."



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Wed, 30 May 2018 03:44:41 +0000
Gerrit-HasComments: Yes

Reply via email to