You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2015/02/04 21:50:50 UTC
mesos git commit: Fixed MESOS-2319 by creating the temporary file
under the same base directory.
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 <yu...@gmail.com>
Authored: Wed Feb 4 11:37:38 2015 -0800
Committer: Jie Yu <yu...@gmail.com>
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();