> On 2012-03-26 23:09:55, Matei Zaharia wrote: > > README, line 141 > > <https://reviews.apache.org/r/4491/diff/1/?file=95961#file95961line141> > > > > But the slaves-env.sh file needs to be on all the nodes, right? If the > > intent is to ship the options automatically, that would make it hard for > > the user to configure, say, different numbers of CPUs and memory per host, > > or a different LIBPROCESS_IP, or stuff like that. > > Andy Konwinski wrote: > I chatted with Matei offline about this. The current flow does not ship > the slaves-env.sh file over. If people wanted to set up unique per-machine > slave configurations, they can create a config file somewhere on each such > machine containing the special settings for that machine, and then set their > slaves-env.sh to point to that via the > --conf=/location/of/mesos/conf/file/on/slave/machines, and the values set in > those files would override anything passed over via the deploy scripts as env > variables. > > This raises the important issue of what the order of precedence is for > options that are set in multipole of {env variable, configuration file, > command line} (note that this is not a new issue). > https://github.com/mesos/mesos/wiki/Configuration currently says > "Configuration values are searched for first on the command line, then in the > environment, and then in the config file." but we should expand that comment > to say explicitly what happens if a variable is set in more than one place > (maybe even give an example). > > Matei Zaharia wrote: > Just to make it clear, the goal of the deploy scripts was never to ship > the configuration automatically to all the machines. The old deploy-to-slaves > script happened to do that, but its main purpose is to let people developing > Mesos modify and recompile the code locally, then sync it to all the > machines. I think it's fine to tell the user to configure each machine > separately -- in fact they won't have to do a single thing by default because > most users don't set any config settings on slaves. Otherwise, the mechanisms > for "global" and "local" configuration should be consistent (both environment > variables or both files). > > Matei Zaharia wrote: > By the way, my suggestion with this kind of stuff is that if you don't > want to spend a long time thinking about how to do it, find another project > that uses a similar configuration process, and copy what it does. Otherwise, > you will repeat mistakes that others have found and fixed in the past. This > is why the old scripts were based on per-machine config files in a key-value > pair format -- because a lot of other cluster software does that. Same thing > with the localstatedir / .template file issue. If in doubt go with .template > and tell the user to copy it. > > Benjamin Hindman wrote: > Woah, hold on a second. I was trying to get the _same_ semantics as the > _old_ scripts. In the old scripts, if you had a local configuration file > which had the 'url' field set the deploy scripts would actually use that when > launching masters and slaves. That is to say, local configuration was used to > create command line options for remote execution! I've stayed consistent with > this methodology, just using mesos-*env.sh and environment variables rather > than a configuration file in order to deal with naming conflicts of options > between slaves and masters. > > But to be clear, as Andy mentioned, by setting MESOS_CONF any local > configuration will get overridden by remote hosts configuration files. I > would have been happy not implementing this, but I was _trying_ to keep the > old workflow.
Got it. I think the old scripts were doing this wrong for the URL then, by assuming that the URL is the same on all the machines. The sudo was similarly a special case because of the way we launched commands (we needed to run ssh <node> sudo <command> instead of doing ssh <node> <command> and having the <command> script call sudo. I don't think this is the right way to go. Again, I think setting the --conf flag to specify local options is a bad idea because it requires the user to learn two forms of configuration. > On 2012-03-26 23:09:55, Matei Zaharia wrote: > > src/deploy/mesos-start-masters.sh.in, line 36 > > <https://reviews.apache.org/r/4491/diff/1/?file=95974#file95974line36> > > > > What does this do? If you're trying to ship the env variables from the > > local config files to the machine, shouldn't you do export ${var} = value? > > Also, gotta be careful with quoting here. > > Andy Konwinski wrote: > I believe the `var` command prints one "KEY=value" pair per line so the > way Ben has it should work. Perhaps renaming the bash variable here from var > to var_and_value or var_value_pair would make it more clear. > > Also, I think you're right Matei that the way we're passing the > environment variables right now might break if any of those environment > variables contain double quotes (e.g. <code>export MESOS_CONFIG_VAR="say > \"hi\""</code>). One thing we could do is just state in the documentation (on > the wiki, in the README, and in the *-env.sh files themselves) that users > should not include any strings with any quotes (i.e. \") in them. > > Matei Zaharia wrote: > This is pretty hacky IMO -- would prefer to avoid this. Just having it in > the documentation doesn't mean it will be easy to find (a user might still > spend a while debugging). The problem is not only quotes but also spaces in > the variables, because env doesn't quite its outputs. For example: > > matei@mbk:~$ export MYVAR="hello world" > matei@mbk:~$ env | grep MYVAR > MYVAR=hello world > > Benjamin Hindman wrote: > I've updated the scripts to handle the quoting better. Again, this is an > attempt to match the old semantics. However, in the old semantics, not all > the configuration options specified in the local configuration would be used, > only those that had been added to the deploy scripts and determined via > 'mesos-getconf'. This sounds horrible to me, because it means as we add > configuration options we also need to remember to add them into the deploy > scripts (just satisfying all of the current options would have required a lot > more work). As mentioned above, this was more of a mistake for URL and deploy_with_sudo than an intended goal. The intent was that you can configure each machine separately. - Matei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4491/#review6383 ----------------------------------------------------------- On 2012-03-27 04:38:04, Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4491/ > ----------------------------------------------------------- > > (Updated 2012-03-27 04:38:04) > > > 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 090c07a > src/conf/deploy-env.sh.template a4565e2 > src/configurator/configurator.hpp ecd1365 > src/deploy/deploy-to-slaves 0e10486 > src/deploy/mesos-common-env.sh.template PRE-CREATION > 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 c3dfda2 > src/deploy/mesos-masters-env.sh.template PRE-CREATION > src/deploy/mesos-slaves-env.sh.template PRE-CREATION > 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 f31212b > > Diff: https://reviews.apache.org/r/4491/diff > > > Testing > ------- > > N/A > > > Thanks, > > Benjamin > >
