Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12517 )

Change subject: [tools] Support running the master and tablet server via the 
kudu binary
......................................................................


Patch Set 11:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12517/13//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12517/13//COMMIT_MSG@16
PS13, Line 16: appoximately
> approximately
Done


http://gerrit.cloudera.org:8080/#/c/12517/13//COMMIT_MSG@29
PS13, Line 29: neccessary
> necessary
Done


http://gerrit.cloudera.org:8080/#/c/12517/11/src/kudu/master/master_runner.cc
File src/kudu/master/master_runner.cc:

http://gerrit.cloudera.org:8080/#/c/12517/11/src/kudu/master/master_runner.cc@68
PS11, Line 68: Explicitly set the flag along with the default as this should 
not be overridden.
> I'm not sure I understand this comment.  Does it say that the value for the
SetCommandLineOptionWithMode when using the mode SET_FLAGS_DEFAULT will always 
set the default, but will only set the value if a value hasn't already been 
set. You can see this description on the SET_FLAGS_DEFAULT docs.

Because we want to ensure evict_failed_followers is true regardless of previous 
settings we set it here. I will update the docs.


http://gerrit.cloudera.org:8080/#/c/12517/11/src/kudu/tserver/tablet_server_runner.cc
File src/kudu/tserver/tablet_server_runner.cc:

http://gerrit.cloudera.org:8080/#/c/12517/11/src/kudu/tserver/tablet_server_runner.cc@64
PS11, Line 64:   CHECK_NE("", 
SetCommandLineOptionWithMode("force_block_cache_capacity",
             :                                             "false", 
gflags::SET_FLAGS_DEFAULT));
> nit: do you want to set the force_block_cache_capacity flag explicitly to t
I am not sure this one needs to be so strictly enforced. It sounds like this is 
mostly about validating configs, but still could be useful to override in 
extraneous situations. At least not dangerous to override.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3717cbac930b3506a76f7a51388c64afbcbb480e
Gerrit-Change-Number: 12517
Gerrit-PatchSet: 11
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Thu, 21 Feb 2019 02:42:28 +0000
Gerrit-HasComments: Yes

Reply via email to