Repository: mesos Updated Branches: refs/heads/master 8c72937db -> d29eec5f0
Added ABORT and CHECK_* usage in Mesos. Review: https://reviews.apache.org/r/18551 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d29eec5f Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d29eec5f Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d29eec5f Branch: refs/heads/master Commit: d29eec5f0f53673334d77e815283452f56a87027 Parents: 8c72937 Author: Dominic Hamon <[email protected]> Authored: Thu Mar 6 11:46:03 2014 -0800 Committer: Benjamin Hindman <[email protected]> Committed: Thu Mar 6 11:46:03 2014 -0800 ---------------------------------------------------------------------- src/exec/exec.cpp | 12 +++------ .../org_apache_mesos_state_AbstractState.cpp | 17 +++++++------ src/log/leveldb.cpp | 2 ++ src/log/log.cpp | 8 +++--- src/log/network.hpp | 4 +-- src/log/recover.cpp | 2 +- src/slave/containerizer/cgroups_launcher.cpp | 15 +++-------- src/slave/containerizer/launcher.cpp | 6 +---- src/slave/containerizer/mesos_containerizer.cpp | 26 ++++++-------------- src/slave/slave.cpp | 7 ++---- 10 files changed, 35 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/exec/exec.cpp ---------------------------------------------------------------------- diff --git a/src/exec/exec.cpp b/src/exec/exec.cpp index 0ab5cc2..346e39f 100644 --- a/src/exec/exec.cpp +++ b/src/exec/exec.cpp @@ -645,9 +645,7 @@ Status MesosExecutorDriver::start() // Get slave PID from environment. value = os::getenv("MESOS_SLAVE_PID"); slave = UPID(value); - if (!slave) { - fatal("Cannot parse MESOS_SLAVE_PID '%s'", value.c_str()); - } + CHECK(slave) << "Cannot parse MESOS_SLAVE_PID '" << value << "'"; // Get slave ID from environment. value = os::getenv("MESOS_SLAVE_ID"); @@ -683,11 +681,9 @@ Status MesosExecutorDriver::start() if (!value.empty()) { Try<Duration> _recoveryTimeout = Duration::parse(value); - if (_recoveryTimeout.isError()) { - fatal("Cannot parse MESOS_RECOVERY_TIMEOUT '%s': %s", - value.c_str(), - _recoveryTimeout.error().c_str()); - } + CHECK_SOME(_recoveryTimeout) + << "Cannot parse MESOS_RECOVERY_TIMEOUT '" << value << "': " + << _recoveryTimeout.error(); recoveryTimeout = _recoveryTimeout.get(); } http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/java/jni/org_apache_mesos_state_AbstractState.cpp ---------------------------------------------------------------------- diff --git a/src/java/jni/org_apache_mesos_state_AbstractState.cpp b/src/java/jni/org_apache_mesos_state_AbstractState.cpp index 0c7aebf..154bbc6 100644 --- a/src/java/jni/org_apache_mesos_state_AbstractState.cpp +++ b/src/java/jni/org_apache_mesos_state_AbstractState.cpp @@ -3,6 +3,7 @@ #include <string> #include <vector> +#include <process/check.hpp> #include <process/future.hpp> #include <stout/duration.hpp> @@ -141,7 +142,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_mesos_state_AbstractState__1_1fetch_1g return NULL; } - CHECK(future->isReady()); + CHECK_READY(*future); Variable* variable = new Variable(future->get()); @@ -190,7 +191,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_mesos_state_AbstractState__1_1fetch_1g return NULL; } - CHECK(future->isReady()); + CHECK_READY(*future); Variable* variable = new Variable(future->get()); // Variable variable = new Variable(); @@ -326,7 +327,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_mesos_state_AbstractState__1_1store_1g return NULL; } - CHECK(future->isReady()); + CHECK_READY(*future); if (future->get().isSome()) { Variable* variable = new Variable(future->get().get()); @@ -379,7 +380,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_mesos_state_AbstractState__1_1store_1g return NULL; } - CHECK(future->isReady()); + CHECK_READY(*future); if (future->get().isSome()) { Variable* variable = new Variable(future->get().get()); @@ -519,7 +520,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_mesos_state_AbstractState__1_1expunge_ return NULL; } - CHECK(future->isReady()); + CHECK_READY(*future); if (future->get()) { jclass clazz = env->FindClass("java/lang/Boolean"); @@ -565,7 +566,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_mesos_state_AbstractState__1_1expunge_ return NULL; } - CHECK(future->isReady()); + CHECK_READY(*future); if (future->get()) { jclass clazz = env->FindClass("java/lang/Boolean"); @@ -692,7 +693,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_mesos_state_AbstractState__1_1names_1g return NULL; } - CHECK(future->isReady()); + CHECK_READY(*future); // List names = new ArrayList(); jclass clazz = env->FindClass("java/util/ArrayList"); @@ -748,7 +749,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_mesos_state_AbstractState__1_1names_1g return NULL; } - CHECK(future->isReady()); + CHECK_READY(*future); // List names = new ArrayList(); clazz = env->FindClass("java/util/ArrayList"); http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/log/leveldb.cpp ---------------------------------------------------------------------- diff --git a/src/log/leveldb.cpp b/src/log/leveldb.cpp index f8c0f65..7811f2b 100644 --- a/src/log/leveldb.cpp +++ b/src/log/leveldb.cpp @@ -18,6 +18,8 @@ #include <google/protobuf/io/zero_copy_stream_impl.h> +#include <glog/logging.h> + #include <leveldb/comparator.h> #include <leveldb/write_batch.h> http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/log/log.cpp ---------------------------------------------------------------------- diff --git a/src/log/log.cpp b/src/log/log.cpp index 2d3a0cf..c427920 100644 --- a/src/log/log.cpp +++ b/src/log/log.cpp @@ -460,7 +460,7 @@ Future<Log::Position> LogReaderProcess::beginning() Future<Log::Position> LogReaderProcess::_beginning() { - CHECK(recovering.isReady()); + CHECK_READY(recovering); return recovering.get()->beginning() .then(lambda::bind(&Self::position, lambda::_1)); @@ -475,7 +475,7 @@ Future<Log::Position> LogReaderProcess::ending() Future<Log::Position> LogReaderProcess::_ending() { - CHECK(recovering.isReady()); + CHECK_READY(recovering); return recovering.get()->ending() .then(lambda::bind(&Self::position, lambda::_1)); @@ -494,7 +494,7 @@ Future<list<Log::Entry> > LogReaderProcess::_read( const Log::Position& from, const Log::Position& to) { - CHECK(recovering.isReady()); + CHECK_READY(recovering); return recovering.get()->read(from.value, to.value) .then(defer(self(), &Self::__read, from, to, lambda::_1)); @@ -628,7 +628,7 @@ Future<Option<Log::Position> > LogWriterProcess::_elect() delete coordinator; error = None(); - CHECK(recovering.isReady()); + CHECK_READY(recovering); coordinator = new Coordinator(quorum, recovering.get(), network); http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/log/network.hpp ---------------------------------------------------------------------- diff --git a/src/log/network.hpp b/src/log/network.hpp index 74dc200..2befea4 100644 --- a/src/log/network.hpp +++ b/src/log/network.hpp @@ -400,7 +400,7 @@ inline void ZooKeeperNetwork::watched( LOG(FATAL) << "Failed to watch ZooKeeper group: " << memberships.failure(); } - CHECK(memberships.isReady()); // Not expecting Group to discard futures. + CHECK_READY(memberships); // Not expecting Group to discard futures. LOG(INFO) << "ZooKeeper group memberships changed"; @@ -429,7 +429,7 @@ inline void ZooKeeperNetwork::collected( return; } - CHECK(datas.isReady()); // Not expecting collect to discard futures. + CHECK_READY(datas); // Not expecting collect to discard futures. std::set<process::UPID> pids; http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/log/recover.cpp ---------------------------------------------------------------------- diff --git a/src/log/recover.cpp b/src/log/recover.cpp index a5819ec..e611a4e 100644 --- a/src/log/recover.cpp +++ b/src/log/recover.cpp @@ -212,7 +212,7 @@ private: void received(const Future<RecoverResponse>& future) { // Enforced by the select semantics. - CHECK(future.isReady()); + CHECK_READY(future); // Remove this future from 'responses' so that we do not listen on // it the next time we invoke select. http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/slave/containerizer/cgroups_launcher.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/cgroups_launcher.cpp b/src/slave/containerizer/cgroups_launcher.cpp index a9b0108..39f0e4c 100644 --- a/src/slave/containerizer/cgroups_launcher.cpp +++ b/src/slave/containerizer/cgroups_launcher.cpp @@ -16,10 +16,9 @@ * limitations under the License. */ -#include <unistd.h> - #include <vector> +#include <stout/abort.hpp> #include <stout/hashset.hpp> #include <stout/path.hpp> #include <stout/unreachable.hpp> @@ -187,12 +186,8 @@ Try<pid_t> CgroupsLauncher::fork( while ((len = read(pipes[0], &buf, sizeof(buf))) == -1 && errno == EINTR); if (len != sizeof(buf)) { - const char* message = "Failed to synchronize with parent"; - // Ignore the return value from write() to silence compiler warning. - while (write(STDERR_FILENO, message, strlen(message)) == -1 && - errno == EINTR); os::close(pipes[0]); - _exit(1); + ABORT("Failed to synchronize with parent"); } os::close(pipes[0]); @@ -200,11 +195,7 @@ Try<pid_t> CgroupsLauncher::fork( // This function should exec() and therefore not return. inChild(); - const char* message = "Child failed to exec"; - while (write(STDERR_FILENO, message, strlen(message)) == -1 && - errno == EINTR); - - _exit(1); + ABORT("Child failed to exec"); } // Parent. http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/slave/containerizer/launcher.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/launcher.cpp b/src/slave/containerizer/launcher.cpp index 2361a20..fb0c461 100644 --- a/src/slave/containerizer/launcher.cpp +++ b/src/slave/containerizer/launcher.cpp @@ -109,11 +109,7 @@ Try<pid_t> PosixLauncher::fork( // This function should exec() and therefore not return. inChild(); - const char* message = "Child failed to exec"; - while (write(STDERR_FILENO, message, strlen(message)) == -1 && - errno == EINTR); - - _exit(1); + ABORT("Child failed to exec"); } // parent. http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/slave/containerizer/mesos_containerizer.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos_containerizer.cpp b/src/slave/containerizer/mesos_containerizer.cpp index 6d990cb..9bf9829 100644 --- a/src/slave/containerizer/mesos_containerizer.cpp +++ b/src/slave/containerizer/mesos_containerizer.cpp @@ -229,18 +229,6 @@ Future<Nothing> MesosContainerizerProcess::_recover( } -// Log the message and then exit(1) in an async-signal-safe manner. -// TODO(idownes): Move this into stout, possibly replacing its fatal(), and -// support multiple messages to write out. -void asyncSafeFatal(const char* message) -{ - // Ignore the return value from write() to silence compiler warning. - while (write(STDERR_FILENO, message, strlen(message)) == -1 && - errno == EINTR); - _exit(1); -} - - // This function is executed by the forked child and should be // async-signal-safe. // TODO(idownes): Several functions used here are not actually @@ -267,7 +255,7 @@ int execute( if (len != sizeof(buf)) { os::close(pipeRead); - asyncSafeFatal("Failed to synchronize with parent"); + ABORT("Failed to synchronize with parent"); } os::close(pipeRead); @@ -275,18 +263,18 @@ int execute( if (user.isSome()) { Try<Nothing> chown = os::chown(user.get(), directory); if (chown.isError()) { - asyncSafeFatal("Failed to chown work directory"); + ABORT("Failed to chown work directory"); } } // Change user if provided. if (user.isSome() && !os::su(user.get())) { - asyncSafeFatal("Failed to change user"); + ABORT("Failed to change user"); } // Enter working directory. if (os::chdir(directory) < 0) { - asyncSafeFatal("Failed to chdir into work directory"); + ABORT("Failed to chdir into work directory"); } // First set up any additional environment variables. @@ -312,10 +300,10 @@ int execute( // directly. if (redirectIO) { if (freopen("stdout", "a", stdout) == NULL) { - asyncSafeFatal("freopen failed"); + ABORT("freopen failed"); } if (freopen("stderr", "a", stderr) == NULL) { - asyncSafeFatal("freopen failed"); + ABORT("freopen failed"); } } @@ -323,7 +311,7 @@ int execute( execl("/bin/sh", "sh", "-c", command.value().c_str(), (char*) NULL); // If we get here, the execv call failed. - asyncSafeFatal("Failed to execute command"); + ABORT("Failed to execute command"); // This should not be reached. return -1; http://git-wip-us.apache.org/repos/asf/mesos/blob/d29eec5f/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index b350df4..6abb95d 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -28,6 +28,7 @@ #include <vector> #include <process/async.hpp> +#include <process/check.hpp> #include <process/defer.hpp> #include <process/delay.hpp> #include <process/dispatch.hpp> @@ -1794,11 +1795,7 @@ void Slave::_statusUpdate( const StatusUpdate& update, const UPID& pid) { - if (!future.isReady()) { - LOG(FATAL) << "Failed to handle status update " << update << ": " - << (future.isFailed() ? future.failure() : "future discarded"); - return; - } + CHECK_READY(future) << "Failed to handle status update " << update; VLOG(1) << "Status update manager successfully handled status update " << update;
