Made HDFS::exists asynchronous. This also fixed a bug in the orignal code: the original code never returns false.
Review: https://reviews.apache.org/r/40806/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3424b1f7 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3424b1f7 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3424b1f7 Branch: refs/heads/master Commit: 3424b1f702da1d0c33d0a3e32569477e31dbb4e8 Parents: b420b39 Author: Jie Yu <[email protected]> Authored: Mon Dec 14 11:42:13 2015 -0800 Committer: Jie Yu <[email protected]> Committed: Tue Dec 15 10:55:33 2015 -0800 ---------------------------------------------------------------------- src/cli/execute.cpp | 11 ++++++++--- src/hdfs/hdfs.cpp | 42 ++++++++++++++++++++++++++++++------------ src/hdfs/hdfs.hpp | 3 ++- 3 files changed, 40 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/3424b1f7/src/cli/execute.cpp ---------------------------------------------------------------------- diff --git a/src/cli/execute.cpp b/src/cli/execute.cpp index ac933b6..eef3b91 100644 --- a/src/cli/execute.cpp +++ b/src/cli/execute.cpp @@ -21,6 +21,7 @@ #include <mesos/scheduler.hpp> #include <mesos/type_utils.hpp> +#include <process/future.hpp> #include <process/owned.hpp> #include <process/pid.hpp> @@ -40,6 +41,7 @@ using namespace mesos; using namespace mesos::internal; +using process::Future; using process::Owned; using process::UPID; @@ -385,9 +387,12 @@ int main(int argc, char** argv) string path = path::join("/", user.get(), flags.package.get()); // Check if the file exists and remove it if we're overwriting. - Try<bool> exists = hdfs.get()->exists(path); - if (exists.isError()) { - cerr << "Failed to check if file exists: " << exists.error() << endl; + Future<bool> exists = hdfs.get()->exists(path); + exists.await(); + + if (!exists.isReady()) { + cerr << "Failed to check if file exists: " + << (exists.isFailed() ? exists.failure() : "discarded") << endl; return EXIT_FAILURE; } else if (exists.get() && flags.overwrite) { Try<Nothing> rm = hdfs.get()->rm(path); http://git-wip-us.apache.org/repos/asf/mesos/blob/3424b1f7/src/hdfs/hdfs.cpp ---------------------------------------------------------------------- diff --git a/src/hdfs/hdfs.cpp b/src/hdfs/hdfs.cpp index e5e7097..1038cc3 100644 --- a/src/hdfs/hdfs.cpp +++ b/src/hdfs/hdfs.cpp @@ -122,22 +122,40 @@ Try<Owned<HDFS>> HDFS::create(const Option<string>& _hadoop) } -Try<bool> HDFS::exists(const string& path) +Future<bool> HDFS::exists(const string& path) { - Try<string> command = strings::format( - "%s fs -test -e '%s'", hadoop, absolutePath(path)); - - CHECK_SOME(command); + Try<Subprocess> s = subprocess( + hadoop, + {"hadoop", "fs", "-test", "-e", absolutePath(path)}, + Subprocess::PATH("/dev/null"), + Subprocess::PIPE(), + Subprocess::PIPE()); + + if (s.isError()) { + return Failure("Failed to execute the subprocess: " + s.error()); + } - // We are piping stderr to stdout so that we can see the error (if - // any) in the logs emitted by `os::shell()` in case of failure. - Try<string> out = os::shell(command.get() + " 2>&1"); + return result(s.get()) + .then([](const CommandResult& result) -> Future<bool> { + if (result.status.isNone()) { + return Failure("Failed to reap the subprocess"); + } - if (out.isError()) { - return Error(out.error()); - } + if (WIFEXITED(result.status.get())) { + int exitCode = WEXITSTATUS(result.status.get()); + if (exitCode == 0) { + return true; + } else if (exitCode == 1) { + return false; + } + } - return true; + return Failure( + "Unexpected result from the subprocess: " + "status='" + stringify(result.status.get()) + "', " + + "stdout='" + result.out + "', " + + "stderr='" + result.err + "'"); + }); } http://git-wip-us.apache.org/repos/asf/mesos/blob/3424b1f7/src/hdfs/hdfs.hpp ---------------------------------------------------------------------- diff --git a/src/hdfs/hdfs.hpp b/src/hdfs/hdfs.hpp index 6bdeedf..aa44903 100644 --- a/src/hdfs/hdfs.hpp +++ b/src/hdfs/hdfs.hpp @@ -19,6 +19,7 @@ #include <string> +#include <process/future.hpp> #include <process/owned.hpp> #include <stout/bytes.hpp> @@ -48,7 +49,7 @@ public: static Try<process::Owned<HDFS>> create( const Option<std::string>& hadoop = None()); - Try<bool> exists(const std::string& path); + process::Future<bool> exists(const std::string& path); Try<Bytes> du(const std::string& path); Try<Nothing> rm(const std::string& path); Try<Nothing> copyFromLocal(const std::string& from, const std::string& to);
