You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2018/05/29 23:33:35 UTC
[kudu-CR] [kudu CLI] kudu tablet set attributes sub-command
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10536
Change subject: [kudu CLI] kudu tablet set_attributes sub-command
......................................................................
[kudu CLI] kudu tablet set_attributes sub-command
Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI
tool. With this tool, it is possible to set/reset the 'promote' and
'replace' attributes in the replica's Raft configuration.
This functionality might be useful for pre-KUDU-2443 clusters running
the 3-4-3 replica management scheme after an attempt to move a replica
of a tablet with replication factor of 1. In that case, the system
would add a non-voter replica and then evict it, doing that over and
over again (that's what KUDU-2443 is about).
Aside from the hapless use-case described above, this functionality
might be useful when removing the 'replace' attribute set on a replica
by the 'kudu tablet change_config move_replica' sub-command before the
move has completed.
Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
---
M src/kudu/tools/tool_action_tablet.cc
1 file changed, 62 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/1
--
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: newchange
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@254
PS3, Line 254: bool has_promote = !GetCommandLineFlagInfoOrDie("attribute_promote").is_default;
> In the future we will likely add attributes. Will we always try to prevent
OK, since it is not harmful to set both, I will remove this check.
--
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: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:01:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 1:
(6 comments)
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.
Done
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().
Done
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 t
It's a good catch -- yes, setting both PROMOTE and REPLACE does not make sense. I updated the code to return InvalidArgument status in that case.
From what I saw in raft_conensus.cc, right now the server side does not impose any restrictions or sanity checks on the attribute combinations.
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 ma
You are right -- this pattern happens a lot. I think it makes sense to have a macro for that, yes. Probably, in a separate changelist?
http://gerrit.cloudera.org:8080/#/c/10536/1/src/kudu/tools/tool_action_tablet.cc@675
PS1, Line 675: /reset
> We can only set.
Done
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 configurati
Right, thanks -- that's a better wording.
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jun 2018 17:28:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has uploaded a new patch set (#4) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
[tools] add 'kudu tablet set_attributes' sub-command
Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI
tool. With this tool, it is possible to set/reset the 'promote' and
'replace' attributes in the replica's Raft configuration.
This functionality might be useful for pre-KUDU-2443 clusters running
the 3-4-3 replica management scheme after an attempt to move a replica
of a tablet with replication factor of 1. In that case, the system
would add a non-voter replica and then evict it, doing that over and
over again (that's what KUDU-2443 is about).
Aside from the hapless use-case described above, this functionality
might be useful when removing the 'replace' attribute set on a replica
by the 'kudu tablet change_config move_replica' sub-command before the
move has completed.
Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_tablet.cc
3 files changed, 201 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/4
--
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: newpatchset
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] add 'kudu tablet set attributes' command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has uploaded a new patch set (#7) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' command
......................................................................
[tools] add 'kudu tablet set_attributes' command
Introduced 'kudu tablet set_attributes' command for the kudu CLI
tool. With this tool, it is possible to set/reset the 'promote' and
'replace' attributes in the replica's Raft configuration.
This functionality might be useful for pre-KUDU-2443 clusters running
the 3-4-3 replica management scheme after an attempt to move a replica
of a tablet with replication factor of 1. In that case, the system
would add a non-voter replica and then evict it, doing that over and
over again (that's what KUDU-2443 is about).
Aside from the hapless use-case described above, this functionality
might be useful when removing the 'replace' attribute set on a replica
by the 'kudu tablet change_config move_replica' command before the
move has completed.
Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_tablet.cc
3 files changed, 212 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/7
--
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: newpatchset
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10536/4/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/4/src/kudu/tools/tool_action_tablet.cc@62
PS4, Line 62: using kudu::client::KuduTablet;
> warning: using decl 'KuduTablet' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/10536/4/src/kudu/tools/tool_action_tablet.cc@68
PS4, Line 68: using kudu::consensus::GetLastOpIdRequestPB;
> warning: using decl 'GetLastOpIdRequestPB' is unused [misc-unused-using-dec
Done
http://gerrit.cloudera.org:8080/#/c/10536/4/src/kudu/tools/tool_action_tablet.cc@69
PS4, Line 69: using kudu::consensus::GetLastOpIdResponsePB;
> warning: using decl 'GetLastOpIdResponsePB' is unused [misc-unused-using-de
Done
--
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: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 17:31:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10536
to look at the new patch set (#2).
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
[tools] add 'kudu tablet set_attributes' sub-command
Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI
tool. With this tool, it is possible to set/reset the 'promote' and
'replace' attributes in the replica's Raft configuration.
This functionality might be useful for pre-KUDU-2443 clusters running
the 3-4-3 replica management scheme after an attempt to move a replica
of a tablet with replication factor of 1. In that case, the system
would add a non-voter replica and then evict it, doing that over and
over again (that's what KUDU-2443 is about).
Aside from the hapless use-case described above, this functionality
might be useful when removing the 'replace' attribute set on a replica
by the 'kudu tablet change_config move_replica' sub-command before the
move has completed.
Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
---
M src/kudu/tools/tool_action_tablet.cc
1 file changed, 70 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/2
--
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: newpatchset
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 5: Verified+1
--
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: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Jun 2018 17:33:55 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 2:
(5 comments)
Needs to be covered in the tool test, and could also use a functional test.
http://gerrit.cloudera.org:8080/#/c/10536/2/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/2/src/kudu/tools/tool_action_tablet.cc@63
PS2, Line 63: DEFINE_bool(attribute_promote, false,
: "The value for the tablet replicas's 'promote' attribute.");
: DEFINE_bool(attribute_replace, false,
: "The value for the tablet replicas's 'replace' attribute.");
We spoke offline about how it doesn't really make sense to set both of these. Instead of separate bool flags, maybe one string flag 'peer_attribute' is better so that a user can't set multiple attributes on the same replica.
http://gerrit.cloudera.org:8080/#/c/10536/2/src/kudu/tools/tool_action_tablet.cc@587
PS2, Line 587: if (promote && replace) {
: // Setting both promote and replace doesn't make sense.
: return Status::InvalidArgument("cannot set both PROMOTE and REPLACE");
: }
With a joint flag this invalid state will be unrepresentable.
http://gerrit.cloudera.org:8080/#/c/10536/2/src/kudu/tools/tool_action_tablet.cc@598
PS2, Line 598: RETURN_NOT_OK(GetTabletLeader(client, tablet_id, &leader_uuid, &leader_hp));
: unique_ptr<ConsensusServiceProxy> proxy;
: RETURN_NOT_OK(BuildProxy(leader_hp.host(), leader_hp.port(), &proxy));
How often do we do this sequence in tools code? Should be a helper to get a consensus proxy to the leader?
In another world, the client would support sending consensus service requests and everything that needed to find a tablet leader would use the same code.
http://gerrit.cloudera.org:8080/#/c/10536/2/src/kudu/tools/tool_action_tablet.cc@614
PS2, Line 614: 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());
: }
This pattern is also very common and could use a macro, I think. RETURN_RPC_NOT_OK? And if the function should return OK if everything was ok, maybe RETURN_RPC_STATUS?
http://gerrit.cloudera.org:8080/#/c/10536/2/src/kudu/tools/tool_action_tablet.cc@688
PS2, Line 688: "UUID of the tablet server hosting the replica"
Does this argument not have a generic description, or does it not fit here?
--
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: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 18:22:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 6:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/10536/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10536/6//COMMIT_MSG@7
PS6, Line 7: sub-command
> Nit: command
Done
http://gerrit.cloudera.org:8080/#/c/10536/6//COMMIT_MSG@9
PS6, Line 9: Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI
> Do we need a corresponding get_attributes command in order to figure out wh
Hm...I thought these attributes were exposed in ksck but don't see any code for it. I think that's how they should be exposed so that will have to be a follow-up.
If it were a separate tool it'd probably be best done as part of a tool that dumps a replica's full consensus info.
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc@1498
PS6, Line 1498: const string replace_flag = "--attribute_replace";
: const string promote_flag = "--attribute_promote";
> Nit: could be const char*, and should be named kFooFlag since they're const
Done
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc@1500
PS6, Line 1500: const MonoDelta timeout = MonoDelta::FromSeconds(10);
> Nit: kTimeout
Done
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc@1582
PS6, Line 1582: string err;
> We're not using err in this call; could we use a different overload of RunK
Done
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc@1605
PS6, Line 1605: ASSERT_TRUE(!peer.attrs().replace());
: ASSERT_TRUE(!peer.attrs().promote());
> ASSERT_FALSE() wouldn't be more idiomatic?
Done
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/tool_action_tablet.cc@62
PS6, Line 62: using kudu::client::KuduTabletServer;
> warning: using decl 'KuduTabletServer' is unused [misc-unused-using-decls]
Done
--
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: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Jun 2018 23:44:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 2:
(1 comment)
I'll take over revving this since Alexey is away for a bit. Mike, would you mind reviewing?
http://gerrit.cloudera.org:8080/#/c/10536/2/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/2/src/kudu/tools/tool_action_tablet.cc@63
PS2, Line 63: DEFINE_bool(attribute_promote, false,
: "The value for the tablet replicas's 'promote' attribute.");
: DEFINE_bool(attribute_replace, false,
: "The value for the tablet replicas's 'replace' attribute.");
> We spoke offline about how it doesn't really make sense to set both of thes
I changed my mind about this after thinking about how things will work if we add more attributes, and also after thinking about how to specify setting vs. unsetting a promote or replace attribute.
--
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: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 21:17:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 5:
(1 comment)
looks good overall
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@254
PS3, Line 254: bool has_promote = !GetCommandLineFlagInfoOrDie("attribute_promote").is_default;
> You and Alexey would have a better idea than me. As I recall when I asked A
In the future we will likely add attributes. Will we always try to prevent the user from setting attribute combinations that are not helpful? What if we wanted to do it to hack around something in the future?
--
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: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 23:08:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' command
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' command
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/tool_action_tablet.cc@255
PS6, Line 255:
> Nit: maybe say --atribute_promote and --attribute_replace, to draw a more d
I think you missed this one.
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/tool_action_tablet.cc@353
PS6, Line 353: t's Raft co
> Nit: isn't this just a 'command'? Why 'sub-command'?
And this one.
--
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: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Jun 2018 00:02:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 6:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/10536/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10536/6//COMMIT_MSG@7
PS6, Line 7: sub-command
Nit: command
Below too.
http://gerrit.cloudera.org:8080/#/c/10536/6//COMMIT_MSG@9
PS6, Line 9: Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI
Do we need a corresponding get_attributes command in order to figure out what attribute changes should be made? Or do the current attributes not matter; we always know, for a given tablet, whether PROMOTE or REPLACE should be set given whatever circumstances?
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc@1498
PS6, Line 1498: const string replace_flag = "--attribute_replace";
: const string promote_flag = "--attribute_promote";
Nit: could be const char*, and should be named kFooFlag since they're constants.
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc@1500
PS6, Line 1500: const MonoDelta timeout = MonoDelta::FromSeconds(10);
Nit: kTimeout
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc@1582
PS6, Line 1582: string err;
We're not using err in this call; could we use a different overload of RunKuduTool?
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/kudu-admin-test.cc@1605
PS6, Line 1605: ASSERT_TRUE(!peer.attrs().replace());
: ASSERT_TRUE(!peer.attrs().promote());
ASSERT_FALSE() wouldn't be more idiomatic?
Above too.
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/tool_action_tablet.cc@255
PS6, Line 255: "must set one of PROMOTE or REPLACE"
Nit: maybe say --atribute_promote and --attribute_replace, to draw a more direct link to the gflags?
http://gerrit.cloudera.org:8080/#/c/10536/6/src/kudu/tools/tool_action_tablet.cc@353
PS6, Line 353: sub-command
Nit: isn't this just a 'command'? Why 'sub-command'?
--
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: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Jun 2018 23:01:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has uploaded a new patch set (#3) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
[tools] add 'kudu tablet set_attributes' sub-command
Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI
tool. With this tool, it is possible to set/reset the 'promote' and
'replace' attributes in the replica's Raft configuration.
This functionality might be useful for pre-KUDU-2443 clusters running
the 3-4-3 replica management scheme after an attempt to move a replica
of a tablet with replication factor of 1. In that case, the system
would add a non-voter replica and then evict it, doing that over and
over again (that's what KUDU-2443 is about).
Aside from the hapless use-case described above, this functionality
might be useful when removing the 'replace' attribute set on a replica
by the 'kudu tablet change_config move_replica' sub-command before the
move has completed.
Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_tablet.cc
3 files changed, 175 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/3
--
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: newpatchset
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] add 'kudu tablet set attributes' command
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' command
......................................................................
Patch Set 7: Code-Review+1
--
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: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Jun 2018 00:10:45 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has uploaded a new patch set (#6) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
[tools] add 'kudu tablet set_attributes' sub-command
Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI
tool. With this tool, it is possible to set/reset the 'promote' and
'replace' attributes in the replica's Raft configuration.
This functionality might be useful for pre-KUDU-2443 clusters running
the 3-4-3 replica management scheme after an attempt to move a replica
of a tablet with replication factor of 1. In that case, the system
would add a non-voter replica and then evict it, doing that over and
over again (that's what KUDU-2443 is about).
Aside from the hapless use-case described above, this functionality
might be useful when removing the 'replace' attribute set on a replica
by the 'kudu tablet change_config move_replica' sub-command before the
move has completed.
Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_tablet.cc
3 files changed, 212 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/6
--
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: newpatchset
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [kudu CLI] kudu tablet set attributes sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 30 May 2018 03:44:41 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@254
PS3, Line 254: return Status::InvalidArgument("cannot set both PROMOTE and REPLACE");
I agree it doesn't really make sense but what's the risk if we don't do this validation?
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@273
PS3, Line 273: change->mutable_peer()->mutable_attrs()->set_promote(promote);
We may likely add additional attributes in the future, so I think if you don't specify a flag the tool should not attempt to set it or change its value.
--
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: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jun 2018 19:26:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has uploaded a new patch set (#5) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
[tools] add 'kudu tablet set_attributes' sub-command
Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI
tool. With this tool, it is possible to set/reset the 'promote' and
'replace' attributes in the replica's Raft configuration.
This functionality might be useful for pre-KUDU-2443 clusters running
the 3-4-3 replica management scheme after an attempt to move a replica
of a tablet with replication factor of 1. In that case, the system
would add a non-voter replica and then evict it, doing that over and
over again (that's what KUDU-2443 is about).
Aside from the hapless use-case described above, this functionality
might be useful when removing the 'replace' attribute set on a replica
by the 'kudu tablet change_config move_replica' sub-command before the
move has completed.
Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_tablet.cc
3 files changed, 201 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/5
--
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: newpatchset
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@66
PS3, Line 66: using kudu::consensus::ConsensusStatePB;
> warning: using decl 'ConsensusStatePB' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@67
PS3, Line 67: using kudu::consensus::GetConsensusStateRequestPB;
> warning: using decl 'GetConsensusStateRequestPB' is unused [misc-unused-usi
Done
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@68
PS3, Line 68: using kudu::consensus::GetConsensusStateResponsePB;
> warning: using decl 'GetConsensusStateResponsePB' is unused [misc-unused-us
Done
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@254
PS3, Line 254: return Status::InvalidArgument("cannot set both PROMOTE and REPLACE");
> I agree it doesn't really make sense but what's the risk if we don't do thi
You and Alexey would have a better idea than me. As I recall when I asked Alexey about it, I think he said the replace "wins" when set on a voter, so in that case it's equivalent to just setting replace.
I think it's a good sanity check since a user shouldn't ever intentionally set both, regardless of how risky it is.
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@273
PS3, Line 273: change->mutable_peer()->mutable_attrs()->set_promote(promote);
> We may likely add additional attributes in the future, so I think if you do
Done
--
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: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jun 2018 21:48:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed a vote on this change.
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
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: deleteVote
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>