You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2019/10/04 01:48:31 UTC

[kudu-CR] KUDU-2069 p6: allow setting tserver states

Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14367


Change subject: KUDU-2069 p6: allow setting tserver states
......................................................................

KUDU-2069 p6: allow setting tserver states

Now that the behavior exists and works as we want it to, this patch
removes the gates that prevented setting tserver states.

Change-Id: If56b3f8b95a497c0b72789e80a86877efa33fc2f
---
M src/kudu/integration-tests/maintenance_mode-itest.cc
M src/kudu/master/master_service.cc
M src/kudu/master/ts_state-test.cc
3 files changed, 0 insertions(+), 15 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If56b3f8b95a497c0b72789e80a86877efa33fc2f
Gerrit-Change-Number: 14367
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>

[kudu-CR] KUDU-2069 p6: allow setting tserver states

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

Change subject: KUDU-2069 p6: allow setting tserver states
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14367/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14367/2//COMMIT_MSG@9
PS2, Line 9: Now that the behavior exists and works as we want it to, this patch
           : removes the gates that prevented setting tserver states.
> Curious if you want to preserve this but invert its default value? That way
Nope. The flag only gates setting the state via RPC, not ignoring the state. If we were to allow toggling this, things could get pretty messy with "permanent" states that can't be undone because the RPC endpoint is blocked.

I suppose we could make a flag do more than what it did before this change, but I don't see the value in it, given it's an opt-in feature.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If56b3f8b95a497c0b72789e80a86877efa33fc2f
Gerrit-Change-Number: 14367
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 05 Oct 2019 01:26:23 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2069 p6: allow setting tserver states

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

Change subject: KUDU-2069 p6: allow setting tserver states
......................................................................

KUDU-2069 p6: allow setting tserver states

Now that the behavior exists and works as we want it to, this patch
removes the gates that prevented setting tserver states.

Change-Id: If56b3f8b95a497c0b72789e80a86877efa33fc2f
Reviewed-on: http://gerrit.cloudera.org:8080/14367
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/integration-tests/maintenance_mode-itest.cc
M src/kudu/master/master_service.cc
M src/kudu/master/ts_state-test.cc
3 files changed, 0 insertions(+), 15 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If56b3f8b95a497c0b72789e80a86877efa33fc2f
Gerrit-Change-Number: 14367
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-2069 p6: allow setting tserver states

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

Change subject: KUDU-2069 p6: allow setting tserver states
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If56b3f8b95a497c0b72789e80a86877efa33fc2f
Gerrit-Change-Number: 14367
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 04 Oct 2019 19:01:21 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2069 p6: allow setting tserver states

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

Change subject: KUDU-2069 p6: allow setting tserver states
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If56b3f8b95a497c0b72789e80a86877efa33fc2f
Gerrit-Change-Number: 14367
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 04 Oct 2019 20:13:53 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2069 p6: allow setting tserver states

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

Change subject: KUDU-2069 p6: allow setting tserver states
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14367/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14367/2//COMMIT_MSG@9
PS2, Line 9: Now that the behavior exists and works as we want it to, this patch
           : removes the gates that prevented setting tserver states.
Curious if you want to preserve this but invert its default value? That way (provided it's not experimental/unsafe) we can use it as a feature flag.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If56b3f8b95a497c0b72789e80a86877efa33fc2f
Gerrit-Change-Number: 14367
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 05 Oct 2019 01:11:17 +0000
Gerrit-HasComments: Yes