> On 2012-03-27 22:43:05, Matei Zaharia wrote: > > This looks good, except for a few minor things: > > 1) If you just run these out of the box, the slave will silently fail to > > start because its "--master" command line flag is not passed. We should > > tell the user to configure this in mesos.conf, or try to set it > > automatically if there is only one slave. > > 2) The previous scripts attempted to automatically find the amount of cores > > and memory available. Should we add that back? > > 3) It's not clear that the master and slaves are logging to anywhere (if > > you just run /sbin/mesos-master for example, it says "Logging to" and does > > not give a path). If they aren't, we should update the docs to say that you > > can turn on logging by setting log_dir. Ditto for the work directory (where > > will that be after a make install? It seems like the default is ./work). > > Benjamin Hindman wrote: > 1) I've updated the README with a reminder about at least the "--master" > flag. > 2) No. I'd rather not do anything implicitly. As long as we are forcing > users to update configuration files, they might as well add this information > in themselves. How difficult this will be for them to do (or the lack of > being able to do this programmatically) is a different story, but I don't > want to tackle that now, before the release. If you have suggestions for this > please create a JIRA where we can brainstorm together! > 3) I've updated the output to say "Logging to <stderr>". I've also moved > the work_dir to default to /tmp/mesos (this is okay for multiple slaves > because each actual work directory for the slaves will be under > /tmp/mesos/slaves/ID/...).
Regarding 2, why not add back the old code (the one that set extra options in https://github.com/apache/mesos/blob/trunk/src/deploy/mesos-daemon)? Why break existing functionality for the release? Regarding 3, just mention that you need to set log_dir to see output when using the deploy scripts, because by default stderr gets redirected to /dev/null. - Matei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4491/#review6455 ----------------------------------------------------------- On 2012-03-27 23:12:32, Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4491/ > ----------------------------------------------------------- > > (Updated 2012-03-27 23:12:32) > > > Review request for mesos, Andy Konwinski and Matei Zaharia. > > > Summary > ------- > > Updated deploy scripts to work with latest version of Mesos. Eliminated > 'deploy-to-slaves' functionality, instead we assume the binaries and > libraries will get installed via other means. Updated the semantics of where > configuration options are gathered from since a single configuration file was > insufficient given conflicting configuration options between binaries (e.g., > mesos-master and mesos-slave). Added a section to the README describing these > scripts. > > > Diffs > ----- > > README 8166b04 > configure.ac 7a0d89f > src/Makefile.am 448410e > src/conf/deploy-env.sh.template a4565e2 > src/configurator/configurator.hpp ecd1365 > src/configurator/configurator.cpp 1f8134c > src/deploy/deploy-to-slaves 0e10486 > src/deploy/mesos-daemon b3144c0 > src/deploy/mesos-daemon.sh.in PRE-CREATION > src/deploy/mesos-deploy-env.sh.template PRE-CREATION > src/deploy/mesos-env.sh 14ad67a > src/deploy/mesos-start-cluster.sh.in PRE-CREATION > src/deploy/mesos-start-masters.sh.in PRE-CREATION > src/deploy/mesos-start-slaves.sh.in PRE-CREATION > src/deploy/mesos-stop-cluster.sh.in PRE-CREATION > src/deploy/mesos-stop-masters.sh.in PRE-CREATION > src/deploy/mesos-stop-slaves.sh.in PRE-CREATION > src/deploy/start-masters a450ee0 > src/deploy/start-mesos 9e5b922 > src/deploy/start-slaves b3b931f > src/deploy/stop-masters f2aa9e2 > src/deploy/stop-mesos 679a09e > src/deploy/stop-slaves 141acd4 > src/master/main.cpp e88315f > > Diff: https://reviews.apache.org/r/4491/diff > > > Testing > ------- > > N/A > > > Thanks, > > Benjamin > >
