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

Change subject: [examples] Example shell scripts to start and stop Kudu cluster
......................................................................


Patch Set 6:

(4 comments)

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

PS1:
> The examples directories are labeled based off of the "integration" they ar
Vlad's scripts bring up a cluster using binaries in your dev tree. The 
Docker-based quickstart doesn't do that, and it's one of the reasons these 
scripts are useful.

Agreed that examples/quickstart isn't a great home given the dev-centric nature 
though. Perhaps inside the existing src/kudu/scripts directory? Maybe in a new 
'cluster' subdirectory?


http://gerrit.cloudera.org:8080/#/c/14192/6/examples/quickstart/scripts/start_kudu.sh
File examples/quickstart/scripts/start_kudu.sh:

http://gerrit.cloudera.org:8080/#/c/14192/6/examples/quickstart/scripts/start_kudu.sh@27
PS6, Line 27: WEBSERVER_DOC_ROOT=$PWD/../../www
Does this work properly if your cwd is somewhere else and you're running the 
script via absolute/relative path? You could address that via something like 
SRC_ROOT=$(readlink -f $(dirname $0)/../..)

For that matter, should we allow the root dir to be configurable, perhaps via 
env variable?


http://gerrit.cloudera.org:8080/#/c/14192/6/examples/quickstart/scripts/start_kudu.sh@31
PS6, Line 31: ./bin/
May also want this to be configurable.


http://gerrit.cloudera.org:8080/#/c/14192/6/examples/quickstart/scripts/start_kudu.sh@95
PS6, Line 95: # Start master server with RPC port 8765 and HTTP port 8764
            : start_master 8765 8764
            : # Start tablet server with RPC port 9870, HTTP port 9871 and 
master port 8765
            : start_tserver tserver-0 9870 9871 8765
            : start_tserver tserver-1 9872 9873 8765
            : start_tserver tserver-2 9874 9875 8765
Maybe you could allow the ports to be configurable via env variables? Seems 
like various systems might have port conflicts depending on what other services 
are running.

In lieu of configuring 8 different ports, maybe require that the HTTP port is 
always equal to the RPC port plus one. And maybe that the tserver ports aren't 
each configurable, but rather you specify just one and the rest are computed 
relative to it.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e7bb66d4cce84d00d346bf50154f449d48e7a54
Gerrit-Change-Number: 14192
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Greg Solovyev <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 10 Sep 2019 04:54:34 +0000
Gerrit-HasComments: Yes

Reply via email to