Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15091 )

Change subject: KUDU-3011 p7: add tool to quiesce server
......................................................................


Patch Set 3:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/15091/3/src/kudu/tools/tool_action_tserver.cc@51
PS3, Line 51: DEFINE_int32(quiescing_request_period_secs, 1, "Period in seconds 
with which "
            :     "the tool will check whether the tserver has finished 
quiescing.");
            :
            : DEFINE_int32(quiescing_timeout_secs, 30, "Amount of time in 
seconds the "
            :     "tool will attempt to wait to quiesce a tablet server before 
giving up.");
It's tempting to say that the user of the tool should script this, and that the 
tool should be a simpler "try_quiesce" one-shot operation. What do you think?


http://gerrit.cloudera.org:8080/#/c/15091/3/src/kudu/tools/tool_action_tserver.cc@378
PS3, Line 378:       .AddAction(std::move(start_quiescing))
Nit: would prefer if the action name and variable matched. Makes it easier to 
learn the action names from either place.


http://gerrit.cloudera.org:8080/#/c/15091/3/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/15091/3/src/kudu/tserver/tablet_service.cc@1030
PS3, Line 1030:   if (!quiesce_tserver) {
              :     context->RespondSuccess();
              :     return;
              :   }
              :
              :   resp->set_num_leaders(server_->num_raft_leaders()->value());
              :   
resp->set_num_active_scanners(server_->scanner_manager()->CountActiveScanners());
              :   LOG(INFO) << Substitute("tserver has $0 leaders and $1 
scanners",
              :       resp->num_leaders(), resp->num_active_scanners());
              :   context->RespondSuccess();
Maybe simpler as:

  if (quiesce_tserver) {
    <set resp stuff>
    LOG(INFO) ...
  }
  context->RespondSuccess();



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89657808cc2b0afc4e1b37ce75937ab12e098d9c
Gerrit-Change-Number: 15091
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 22 Jan 2020 18:19:58 +0000
Gerrit-HasComments: Yes

Reply via email to