Repository: mesos Updated Branches: refs/heads/master 66157a6ce -> 3da05548f
Fixed MESOS-2319 by creating the temporary file under the same base directory. Review: https://reviews.apache.org/r/30635 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3da05548 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3da05548 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3da05548 Branch: refs/heads/master Commit: 3da05548f6bd1d12493b563a826c5ead9f9d2f24 Parents: 66157a6 Author: Jie Yu <[email protected]> Authored: Wed Feb 4 11:37:38 2015 -0800 Committer: Jie Yu <[email protected]> Committed: Wed Feb 4 12:50:16 2015 -0800 ---------------------------------------------------------------------- src/slave/state.hpp | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/3da05548/src/slave/state.hpp ---------------------------------------------------------------------- diff --git a/src/slave/state.hpp b/src/slave/state.hpp index de631fb..f92808a 100644 --- a/src/slave/state.hpp +++ b/src/slave/state.hpp @@ -123,15 +123,26 @@ template <typename T> Try<Nothing> checkpoint(const std::string& path, const T& t) { // Create the base directory. - Try<Nothing> mkdir = os::mkdir(os::dirname(path).get()); + Try<std::string> base = os::dirname(path); + if (base.isError()) { + return Error("Failed to get the base directory path: " + base.error()); + } + + Try<Nothing> mkdir = os::mkdir(base.get()); if (mkdir.isError()) { - return Error("Failed to create directory '" + os::dirname(path).get() + + return Error("Failed to create directory '" + base.get() + "': " + mkdir.error()); } - Try<std::string> temp = os::mktemp(); + // NOTE: We create the temporary file at 'base/XXXXXX' to make sure + // rename below does not cross devices (MESOS-2319). + // + // TODO(jieyu): It's possible that the temporary file becomes + // dangling if slave crashes or restarts while checkpointing. + // Consider adding a way to garbage collect them. + Try<std::string> temp = os::mktemp(path::join(base.get(), "XXXXXX")); if (temp.isError()) { - return Error("Failed to create temporary file: '" + temp.error()); + return Error("Failed to create temporary file: " + temp.error()); } // Now checkpoint the instance of T to the temporary file. @@ -139,8 +150,9 @@ Try<Nothing> checkpoint(const std::string& path, const T& t) if (checkpoint.isError()) { // Try removing the temporary file on error. os::rm(temp.get()); - return Error("Failed to checkpoint to " + temp.get() + "': " + - checkpoint.error()); + + return Error("Failed to write temporary file '" + temp.get() + + "': " + checkpoint.error()); } // Rename the temporary file to the path. @@ -148,8 +160,9 @@ Try<Nothing> checkpoint(const std::string& path, const T& t) if (rename.isError()) { // Try removing the temporary file on error. os::rm(temp.get()); - return Error("Failed to rename '" + temp.get() + "' to '" + path + "': " + - rename.error()); + + return Error("Failed to rename '" + temp.get() + "' to '" + + path + "': " + rename.error()); } return Nothing();
