dizzyd at basho.com
Tue Dec 29 10:46:05 EST 2009
Thanks for the feedback and patches. In the future, it would be helpful if you
could provide rationale for large changesets such as the refactoring of the
scripts. It can difficult to understand the "why" when so much code is
affected; we can do a better job of integrating patches when we understand what
the author intended to accomplish.
All of that said, I've attempted to do a detailed breakdown of your changesets
and provide some (hopefully) helpful feedback.
* Moving common code to riak-common
We can't depend on $(dirname $0) to do the right thing if repackaged for
DEB/RPMS/etc. Perhaps some of the common logic could be moved IIF we maintain
the base variable defs so that we know where to find riak-common.
This is worth
considering and I will look into it.
* Using backticks vs $(..) form
I noticed you replaced a number of backticks with this other form --
is there an
advantage to this?
* Removing wait for process to fully exit
Blocking on the server process to exit is highly desirable in production
environments. Why change it? Also, what value do the async versions of
stop/restart/reboot add? Finally, timeouts on the RPC calls are best practice
in that they ensure you get a useful error if the server times out handling
your RPC request.
* Nodetool is meant to be generic
Nodetool provides a standard set of services for any Erlang embedded node and
is not meant to be customized such as you did for
backup/restore/logger. I concur
that the way we invoke those facilities is a little clunky at the
customizing nodetool is not the right approach.
* Moving user to run as to vm.args
Again, when packaging for DEB/RPMS/etc, the user to run as is likely not
something one would want to be configurable by the end user -- this is why I
placed it in the riak script as a variable. The intention is that the riak
script, like nodetool, is meant to be generic. When being repackaged for a
distributions, the repackaging tool can do a simple SED on the variables at
the top of the riak script and get a fully customized startup script
appropriate for a given distribution.
I'm not too keen on changing the names of these files from
app.config and vm.args to
NODE.[config|args] for a few reasons:
1. Changing the name of the config files does not also ensure that logging and
data directories are also appropriately seperated. It would be easy for
people to think they could just rename/copy the a set of config files and
get seperate nodes -- this would not necssarily be true.
2. The use case for running multiple instances of Riak on a single machine is
not a common one. We provide in the top-level Riak Makefile a target for
constructing multiple nodes (devrel) which is useful for
in production, there will typically only be one software node per machine.
3. Multiple copies of config files increase the possibility of production
running a different configuration than what the operator _thinks_ they are
* Addition of use_fallback_nodes
It's not entirely clear to me why one would want to disable the use of
fallback nodes. This would have the effect of removing a key layer
when the network splits or nodes otherwise become unavailable. Out
what's your use case here?
More information about the riak-users