You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mp...@apache.org on 2018/01/11 00:18:26 UTC
[1/5] mesos git commit: Returned `Try` from `protobuf::read(path)`
rather than `Result`.
Repository: mesos
Updated Branches:
refs/heads/master 39d917cc0 -> 3685c011b
Returned `Try<T>` from `protobuf::read(path)` rather than `Result<T>`.
The path version of `protobuf::read` used to return `Result<T>` and
returned `None` only when the file is empty (`ignorePartial` is always
`false`). The `None` return represents EOF for the "streaming" version
of `protobuf::read` that takes an FD, but for the path version an empty
file when we expected to read `T` is simply an error. Thus, we map the
`None` return to an `Error` for the path version and return a `Try<T>`.
Review: https://reviews.apache.org/r/65021
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4f9cda17
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4f9cda17
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4f9cda17
Branch: refs/heads/master
Commit: 4f9cda17e1a747bc3c4ab3667569304e09600b29
Parents: 39d917c
Author: Michael Park <mp...@apache.org>
Authored: Fri Jan 5 16:49:53 2018 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Wed Jan 10 15:57:01 2018 -0800
----------------------------------------------------------------------
3rdparty/stout/include/stout/protobuf.hpp | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4f9cda17/3rdparty/stout/include/stout/protobuf.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/protobuf.hpp b/3rdparty/stout/include/stout/protobuf.hpp
index f00a048..01d7581 100644
--- a/3rdparty/stout/include/stout/protobuf.hpp
+++ b/3rdparty/stout/include/stout/protobuf.hpp
@@ -340,7 +340,7 @@ Result<T> read(int_fd fd, bool ignorePartial = false, bool undoFailed = false)
// A wrapper function that wraps the above read() with open and
// closing the file.
template <typename T>
-Result<T> read(const std::string& path)
+Try<T> read(const std::string& path)
{
Try<int_fd> fd = os::open(
path,
@@ -358,7 +358,13 @@ Result<T> read(const std::string& path)
// read(). Also an unsuccessful close() doesn't affect the read.
os::close(fd.get());
- return result;
+ if (result.isSome()) {
+ return result.get();
+ }
+
+ // `read(fd)` returning `None` here means that the file is empty.
+ // Since this is a partial read of `T`, we report it as an error.
+ return Error(result.isError() ? result.error() : "Found an empty file");
}
[5/5] mesos git commit: Replaced `os::read` with `state::read`.
Posted by mp...@apache.org.
Replaced `os::read` with `state::read`.
The `state::checkpoint` utility was updated to checkpoint
the automatically downgraded resources. This was done to mitigate
the need to manually invoke `downgradeResources` prior to
checkpointing. `state::read` was introduced to provide a symmetric
functionality for `state::checkpoint`. Specifically, it will perform
upgrade resources upon reading the checkpointed resources state.
This patch updates the previous uses of `os::read` which was used to
read the state that was written by `state::checkpoint(path, string)`.
While there is no functional change, it completes the picture where
`state::read` is used to read state written by `state::checkpoint`.
Review: https://reviews.apache.org/r/65025
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3685c011
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3685c011
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3685c011
Branch: refs/heads/master
Commit: 3685c011bb71da0ba2af75691101ea383eeb2ccd
Parents: 80f6606
Author: Michael Park <mp...@apache.org>
Authored: Sat Jan 6 00:48:58 2018 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Wed Jan 10 15:57:11 2018 -0800
----------------------------------------------------------------------
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 2 +-
src/slave/containerizer/mesos/paths.cpp | 6 +++---
src/slave/state.cpp | 8 ++++----
src/tests/slave_recovery_tests.cpp | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/3685c011/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index deacffe..ba9e20c 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -263,7 +263,7 @@ Try<Nothing> DockerVolumeIsolatorProcess::_recover(
return Nothing();
}
- Try<string> read = os::read(volumesPath);
+ Try<string> read = state::read<string>(volumesPath);
if (read.isError()) {
return Error(
"Failed to read docker volumes checkpoint file '" +
http://git-wip-us.apache.org/repos/asf/mesos/blob/3685c011/src/slave/containerizer/mesos/paths.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/paths.cpp b/src/slave/containerizer/mesos/paths.cpp
index 4784d91..612c967 100644
--- a/src/slave/containerizer/mesos/paths.cpp
+++ b/src/slave/containerizer/mesos/paths.cpp
@@ -94,7 +94,7 @@ Result<pid_t> getContainerPid(
return None();
}
- Try<string> read = os::read(path);
+ Try<string> read = state::read<string>(path);
if (read.isError()) {
return Error("Failed to recover pid of container: " + read.error());
}
@@ -180,7 +180,7 @@ Result<pid_t> getContainerIOSwitchboardPid(
return None();
}
- Try<string> read = os::read(path);
+ Try<string> read = state::read<string>(path);
if (read.isError()) {
return Error("Failed to recover pid of io switchboard: " + read.error());
}
@@ -221,7 +221,7 @@ Result<unix::Address> getContainerIOSwitchboardAddress(
return None();
}
- Try<string> read = os::read(path);
+ Try<string> read = state::read<string>(path);
if (read.isError()) {
return Error("Failed reading '" + path + "': " + read.error());
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/3685c011/src/slave/state.cpp
----------------------------------------------------------------------
diff --git a/src/slave/state.cpp b/src/slave/state.cpp
index 63ae264..ff984e4 100644
--- a/src/slave/state.cpp
+++ b/src/slave/state.cpp
@@ -86,7 +86,7 @@ Try<State> recover(const string& rootDir, bool strict)
const string& bootIdPath = paths::getBootIdPath(rootDir);
if (os::exists(bootIdPath)) {
- Try<string> read = os::read(bootIdPath);
+ Try<string> read = state::read<string>(bootIdPath);
if (read.isError()) {
LOG(WARNING) << "Failed to read '"
<< bootIdPath << "': " << read.error();
@@ -240,7 +240,7 @@ Try<FrameworkState> FrameworkState::recover(
return state;
}
- Try<string> pid = os::read(path);
+ Try<string> pid = state::read<string>(path);
if (pid.isError()) {
message =
@@ -455,7 +455,7 @@ Try<RunState> RunState::recover(
return state;
}
- Try<string> pid = os::read(path);
+ Try<string> pid = state::read<string>(path);
if (pid.isError()) {
message = "Failed to read executor forked pid from '" + path +
@@ -491,7 +491,7 @@ Try<RunState> RunState::recover(
rootDir, slaveId, frameworkId, executorId, containerId);
if (os::exists(path)) {
- pid = os::read(path);
+ pid = state::read<string>(path);
if (pid.isError()) {
message = "Failed to read executor libprocess pid from '" + path +
http://git-wip-us.apache.org/repos/asf/mesos/blob/3685c011/src/tests/slave_recovery_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_recovery_tests.cpp b/src/tests/slave_recovery_tests.cpp
index 4d32a84..607fe5d 100644
--- a/src/tests/slave_recovery_tests.cpp
+++ b/src/tests/slave_recovery_tests.cpp
@@ -115,7 +115,7 @@ TEST_F(SlaveStateTest, CheckpointString)
const string file = "test-file";
slave::state::checkpoint(file, expected);
- EXPECT_SOME_EQ(expected, os::read(file));
+ EXPECT_SOME_EQ(expected, slave::state::read<string>(file));
}
[4/5] mesos git commit: Replaced `protobuf::read` with `state::read`.
Posted by mp...@apache.org.
Replaced `protobuf::read` with `state::read`.
The `state::checkpoint` utility was updated to checkpoint
the automatically downgraded resources. This was done to mitigate
the need to manually invoke `downgradeResources` prior to
checkpointing. `state::read` was introduced to provide a symmetric
functionality for `state::checkpoint`. Specifically, it will perform
upgrade resources upon reading the checkpointed resources state.
This patch updates the previous uses of `protobuf::read` accompanied
by calls to `convertResourceFormat`. Rather than reading the protobufs
then upgrading the resources, we know simply call `state::read` which
performs the reverse operation of `state::checkpoint`.
Review: https://reviews.apache.org/r/65024
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/80f66061
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/80f66061
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/80f66061
Branch: refs/heads/master
Commit: 80f66061e343fa26dd7e3b9613f0fa8e0b9b4a36
Parents: ef30390
Author: Michael Park <mp...@apache.org>
Authored: Fri Jan 5 18:03:42 2018 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Wed Jan 10 15:57:11 2018 -0800
----------------------------------------------------------------------
src/resource_provider/storage/provider.cpp | 19 +++-------------
src/slave/containerizer/mesos/paths.cpp | 23 ++++----------------
.../provisioner/docker/metadata_manager.cpp | 2 +-
.../mesos/provisioner/provisioner.cpp | 2 +-
src/slave/state.cpp | 23 ++++----------------
src/tests/slave_recovery_tests.cpp | 12 +++-------
6 files changed, 16 insertions(+), 65 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/80f66061/src/resource_provider/storage/provider.cpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/storage/provider.cpp b/src/resource_provider/storage/provider.cpp
index c0bf3fa..9a32204 100644
--- a/src/resource_provider/storage/provider.cpp
+++ b/src/resource_provider/storage/provider.cpp
@@ -706,7 +706,7 @@ Future<Nothing> StorageLocalResourceProviderProcess::recoverServices()
if (os::exists(configPath)) {
Try<CSIPluginContainerInfo> config =
- ::protobuf::read<CSIPluginContainerInfo>(configPath);
+ slave::state::read<CSIPluginContainerInfo>(configPath);
if (config.isError()) {
return Failure(
@@ -714,9 +714,6 @@ Future<Nothing> StorageLocalResourceProviderProcess::recoverServices()
configPath + "': " + config.error());
}
- convertResourceFormat(
- config->mutable_resources(), POST_RESERVATION_REFINEMENT);
-
if (getCSIPluginContainerInfo(info, containerId) == config.get()) {
continue;
}
@@ -799,7 +796,7 @@ Future<Nothing> StorageLocalResourceProviderProcess::recoverVolumes()
}
Try<csi::state::VolumeState> volumeState =
- ::protobuf::read<csi::state::VolumeState>(statePath);
+ slave::state::read<csi::state::VolumeState>(statePath);
if (volumeState.isError()) {
return Failure(
@@ -903,7 +900,7 @@ StorageLocalResourceProviderProcess::recoverResourceProviderState()
}
Try<ResourceProviderState> resourceProviderState =
- ::protobuf::read<ResourceProviderState>(statePath);
+ slave::state::read<ResourceProviderState>(statePath);
if (resourceProviderState.isError()) {
return Failure(
@@ -911,16 +908,6 @@ StorageLocalResourceProviderProcess::recoverResourceProviderState()
"': " + resourceProviderState.error());
}
- foreach (
- Operation& operation,
- *resourceProviderState->mutable_operations()) {
- upgradeResources(operation.mutable_info());
- }
-
- convertResourceFormat(
- resourceProviderState->mutable_resources(),
- POST_RESERVATION_REFINEMENT);
-
foreach (const Operation& operation,
resourceProviderState->operations()) {
Try<id::UUID> uuid = id::UUID::fromBytes(operation.uuid().value());
http://git-wip-us.apache.org/repos/asf/mesos/blob/80f66061/src/slave/containerizer/mesos/paths.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/paths.cpp b/src/slave/containerizer/mesos/paths.cpp
index 4cec2d2..4784d91 100644
--- a/src/slave/containerizer/mesos/paths.cpp
+++ b/src/slave/containerizer/mesos/paths.cpp
@@ -23,6 +23,7 @@
#include "common/resources_utils.hpp"
#include "slave/containerizer/mesos/paths.hpp"
+#include "slave/state.hpp"
#ifndef __WINDOWS__
namespace unix = process::network::unix;
@@ -277,16 +278,13 @@ Result<ContainerTermination> getContainerTermination(
}
Try<ContainerTermination> termination =
- ::protobuf::read<ContainerTermination>(path);
+ state::read<ContainerTermination>(path);
if (termination.isError()) {
return Error("Failed to read termination state of container: " +
termination.error());
}
- convertResourceFormat(
- termination->mutable_limited_resources(), POST_RESERVATION_REFINEMENT);
-
return termination;
}
@@ -327,25 +325,12 @@ Result<ContainerConfig> getContainerConfig(
return None();
}
- Try<ContainerConfig> containerConfig =
- ::protobuf::read<ContainerConfig>(path);
-
+ Try<ContainerConfig> containerConfig = state::read<ContainerConfig>(path);
if (containerConfig.isError()) {
return Error("Failed to read launch config of container: " +
containerConfig.error());
}
- convertResourceFormat(
- containerConfig->mutable_executor_info()->mutable_resources(),
- POST_RESERVATION_REFINEMENT);
-
- convertResourceFormat(
- containerConfig->mutable_task_info()->mutable_resources(),
- POST_RESERVATION_REFINEMENT);
-
- convertResourceFormat(
- containerConfig->mutable_resources(), POST_RESERVATION_REFINEMENT);
-
return containerConfig;
}
@@ -436,7 +421,7 @@ Result<ContainerLaunchInfo> getContainerLaunchInfo(
}
Try<ContainerLaunchInfo> containerLaunchInfo =
- ::protobuf::read<ContainerLaunchInfo>(path);
+ state::read<ContainerLaunchInfo>(path);
if (containerLaunchInfo.isError()) {
return Error(
http://git-wip-us.apache.org/repos/asf/mesos/blob/80f66061/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
index 548c524..168f786 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
@@ -256,7 +256,7 @@ Future<Nothing> MetadataManagerProcess::recover()
return Nothing();
}
- Try<Images> images = ::protobuf::read<Images>(storedImagesPath);
+ Try<Images> images = state::read<Images>(storedImagesPath);
if (images.isError()) {
return Failure("Failed to read images from '" + storedImagesPath + "' " +
images.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/80f66061/src/slave/containerizer/mesos/provisioner/provisioner.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/provisioner.cpp b/src/slave/containerizer/mesos/provisioner/provisioner.cpp
index 57a437b..39eea34 100644
--- a/src/slave/containerizer/mesos/provisioner/provisioner.cpp
+++ b/src/slave/containerizer/mesos/provisioner/provisioner.cpp
@@ -429,7 +429,7 @@ Future<Nothing> ProvisionerProcess::recover(
VLOG(1) << "Layers path '" << path << "' is missing for container' "
<< containerId << "'";
} else {
- Try<ContainerLayers> layers = ::protobuf::read<ContainerLayers>(path);
+ Try<ContainerLayers> layers = state::read<ContainerLayers>(path);
if (layers.isError()) {
return Failure(
"Failed to recover layers for container '" +
http://git-wip-us.apache.org/repos/asf/mesos/blob/80f66061/src/slave/state.cpp
----------------------------------------------------------------------
diff --git a/src/slave/state.cpp b/src/slave/state.cpp
index 0bc0cca..63ae264 100644
--- a/src/slave/state.cpp
+++ b/src/slave/state.cpp
@@ -151,7 +151,7 @@ Try<SlaveState> SlaveState::recover(
return state;
}
- Try<SlaveInfo> slaveInfo = ::protobuf::read<SlaveInfo>(path);
+ Try<SlaveInfo> slaveInfo = state::read<SlaveInfo>(path);
if (slaveInfo.isError()) {
const string& message = "Failed to read agent info from '" + path + "': " +
slaveInfo.error();
@@ -164,10 +164,6 @@ Try<SlaveState> SlaveState::recover(
}
}
- convertResourceFormat(
- slaveInfo.get().mutable_resources(),
- POST_RESERVATION_REFINEMENT);
-
state.info = slaveInfo.get();
// Find the frameworks.
@@ -219,9 +215,7 @@ Try<FrameworkState> FrameworkState::recover(
return state;
}
- const Try<FrameworkInfo> frameworkInfo =
- ::protobuf::read<FrameworkInfo>(path);
-
+ const Try<FrameworkInfo> frameworkInfo = state::read<FrameworkInfo>(path);
if (frameworkInfo.isError()) {
message = "Failed to read framework info from '" + path + "': " +
frameworkInfo.error();
@@ -379,7 +373,7 @@ Try<ExecutorState> ExecutorState::recover(
return state;
}
- Try<ExecutorInfo> executorInfo = ::protobuf::read<ExecutorInfo>(path);
+ Try<ExecutorInfo> executorInfo = state::read<ExecutorInfo>(path);
if (executorInfo.isError()) {
message = "Failed to read executor info from '" + path + "': " +
executorInfo.error();
@@ -393,10 +387,6 @@ Try<ExecutorState> ExecutorState::recover(
}
}
- convertResourceFormat(
- executorInfo.get().mutable_resources(),
- POST_RESERVATION_REFINEMENT);
-
state.info = executorInfo.get();
return state;
@@ -572,8 +562,7 @@ Try<TaskState> TaskState::recover(
return state;
}
- Try<Task> task = ::protobuf::read<Task>(path);
-
+ Try<Task> task = state::read<Task>(path);
if (task.isError()) {
message = "Failed to read task info from '" + path + "': " + task.error();
@@ -586,10 +575,6 @@ Try<TaskState> TaskState::recover(
}
}
- convertResourceFormat(
- task.get().mutable_resources(),
- POST_RESERVATION_REFINEMENT);
-
state.info = task.get();
// Read the status updates.
http://git-wip-us.apache.org/repos/asf/mesos/blob/80f66061/src/tests/slave_recovery_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_recovery_tests.cpp b/src/tests/slave_recovery_tests.cpp
index fc26987..4d32a84 100644
--- a/src/tests/slave_recovery_tests.cpp
+++ b/src/tests/slave_recovery_tests.cpp
@@ -128,7 +128,7 @@ TEST_F(SlaveStateTest, CheckpointProtobufMessage)
const string file = "slave.id";
slave::state::checkpoint(file, expected);
- const Try<SlaveID> actual = ::protobuf::read<SlaveID>(file);
+ const Try<SlaveID> actual = slave::state::read<SlaveID>(file);
ASSERT_SOME(actual);
EXPECT_SOME_EQ(expected, actual);
@@ -144,16 +144,10 @@ TEST_F(SlaveStateTest, CheckpointRepeatedProtobufMessages)
const string file = "resources-file";
slave::state::checkpoint(file, expected);
- Try<RepeatedPtrField<Resource>> actual =
- ::protobuf::read<RepeatedPtrField<Resource>>(file);
+ const Try<RepeatedPtrField<Resource>> actual =
+ slave::state::read<RepeatedPtrField<Resource>>(file);
ASSERT_SOME(actual);
-
- // We convert the `actual` back to "post-reservation-refinement"
- // since `state::checkpoint` always downgrades whatever resources
- // are downgradable.
- convertResourceFormat(&actual.get(), POST_RESERVATION_REFINEMENT);
-
EXPECT_SOME_EQ(expected, actual);
}
[2/5] mesos git commit: Updated uses of `protobuf::read(path)` which
now returns `Try`.
Posted by mp...@apache.org.
Updated uses of `protobuf::read(path)` which now returns `Try<T>`.
Since the path version of `protobuf::read` now returns `Try<T>`,
many of the existing code is removed and/or simplified.
Review: https://reviews.apache.org/r/65022
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fda054b5
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fda054b5
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fda054b5
Branch: refs/heads/master
Commit: fda054b50ff7cdd2d7a60d31cfe24ce42bfbfaa5
Parents: 4f9cda1
Author: Michael Park <mp...@apache.org>
Authored: Fri Jan 5 17:44:41 2018 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Wed Jan 10 15:57:10 2018 -0800
----------------------------------------------------------------------
src/resource_provider/storage/provider.cpp | 160 +++++++++----------
src/slave/containerizer/mesos/paths.cpp | 30 ++--
.../provisioner/docker/metadata_manager.cpp | 8 +-
.../mesos/provisioner/provisioner.cpp | 20 ++-
src/slave/state.cpp | 38 +----
src/tests/protobuf_io_tests.cpp | 9 +-
src/tests/slave_recovery_tests.cpp | 6 +-
7 files changed, 111 insertions(+), 160 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/fda054b5/src/resource_provider/storage/provider.cpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/storage/provider.cpp b/src/resource_provider/storage/provider.cpp
index 55f2e66..c0bf3fa 100644
--- a/src/resource_provider/storage/provider.cpp
+++ b/src/resource_provider/storage/provider.cpp
@@ -705,7 +705,7 @@ Future<Nothing> StorageLocalResourceProviderProcess::recoverServices()
containerId);
if (os::exists(configPath)) {
- Result<CSIPluginContainerInfo> config =
+ Try<CSIPluginContainerInfo> config =
::protobuf::read<CSIPluginContainerInfo>(configPath);
if (config.isError()) {
@@ -714,13 +714,11 @@ Future<Nothing> StorageLocalResourceProviderProcess::recoverServices()
configPath + "': " + config.error());
}
- if (config.isSome()) {
- convertResourceFormat(
- config->mutable_resources(), POST_RESERVATION_REFINEMENT);
+ convertResourceFormat(
+ config->mutable_resources(), POST_RESERVATION_REFINEMENT);
- if (getCSIPluginContainerInfo(info, containerId) == config.get()) {
- continue;
- }
+ if (getCSIPluginContainerInfo(info, containerId) == config.get()) {
+ continue;
}
}
}
@@ -800,7 +798,7 @@ Future<Nothing> StorageLocalResourceProviderProcess::recoverVolumes()
continue;
}
- Result<csi::state::VolumeState> volumeState =
+ Try<csi::state::VolumeState> volumeState =
::protobuf::read<csi::state::VolumeState>(statePath);
if (volumeState.isError()) {
@@ -809,69 +807,67 @@ Future<Nothing> StorageLocalResourceProviderProcess::recoverVolumes()
volumeState.error());
}
- if (volumeState.isSome()) {
- volumes.put(volumeId, std::move(volumeState.get()));
-
- Future<Nothing> recovered = Nothing();
+ volumes.put(volumeId, std::move(volumeState.get()));
- switch (volumes.at(volumeId).state.state()) {
- case csi::state::VolumeState::CREATED:
- case csi::state::VolumeState::NODE_READY: {
- break;
- }
- case csi::state::VolumeState::PUBLISHED: {
- if (volumes.at(volumeId).state.boot_id() != bootId) {
- // The node has been restarted since the volume is mounted,
- // so it is no longer in the `PUBLISHED` state.
- volumes.at(volumeId).state.set_state(
- csi::state::VolumeState::NODE_READY);
- volumes.at(volumeId).state.clear_boot_id();
- checkpointVolumeState(volumeId);
- }
- break;
- }
- case csi::state::VolumeState::CONTROLLER_PUBLISH: {
- recovered =
- volumes.at(volumeId).sequence->add(std::function<Future<Nothing>()>(
- defer(self(), &Self::controllerPublish, volumeId)));
- break;
- }
- case csi::state::VolumeState::CONTROLLER_UNPUBLISH: {
- recovered =
- volumes.at(volumeId).sequence->add(std::function<Future<Nothing>()>(
- defer(self(), &Self::controllerUnpublish, volumeId)));
- break;
- }
- case csi::state::VolumeState::NODE_PUBLISH: {
- recovered =
- volumes.at(volumeId).sequence->add(std::function<Future<Nothing>()>(
- defer(self(), &Self::nodePublish, volumeId)));
- break;
- }
- case csi::state::VolumeState::NODE_UNPUBLISH: {
- recovered =
- volumes.at(volumeId).sequence->add(std::function<Future<Nothing>()>(
- defer(self(), &Self::nodeUnpublish, volumeId)));
- break;
- }
- case csi::state::VolumeState::UNKNOWN: {
- recovered = Failure(
- "Volume '" + volumeId + "' is in " +
- stringify(volumes.at(volumeId).state.state()) + " state");
- }
+ Future<Nothing> recovered = Nothing();
- // NOTE: We avoid using a default clause for the following
- // values in proto3's open enum to enable the compiler to detect
- // missing enum cases for us. See:
- // https://github.com/google/protobuf/issues/3917
- case google::protobuf::kint32min:
- case google::protobuf::kint32max: {
- UNREACHABLE();
+ switch (volumes.at(volumeId).state.state()) {
+ case csi::state::VolumeState::CREATED:
+ case csi::state::VolumeState::NODE_READY: {
+ break;
+ }
+ case csi::state::VolumeState::PUBLISHED: {
+ if (volumes.at(volumeId).state.boot_id() != bootId) {
+ // The node has been restarted since the volume is mounted,
+ // so it is no longer in the `PUBLISHED` state.
+ volumes.at(volumeId).state.set_state(
+ csi::state::VolumeState::NODE_READY);
+ volumes.at(volumeId).state.clear_boot_id();
+ checkpointVolumeState(volumeId);
}
+ break;
+ }
+ case csi::state::VolumeState::CONTROLLER_PUBLISH: {
+ recovered =
+ volumes.at(volumeId).sequence->add(std::function<Future<Nothing>()>(
+ defer(self(), &Self::controllerPublish, volumeId)));
+ break;
+ }
+ case csi::state::VolumeState::CONTROLLER_UNPUBLISH: {
+ recovered =
+ volumes.at(volumeId).sequence->add(std::function<Future<Nothing>()>(
+ defer(self(), &Self::controllerUnpublish, volumeId)));
+ break;
+ }
+ case csi::state::VolumeState::NODE_PUBLISH: {
+ recovered =
+ volumes.at(volumeId).sequence->add(std::function<Future<Nothing>()>(
+ defer(self(), &Self::nodePublish, volumeId)));
+ break;
+ }
+ case csi::state::VolumeState::NODE_UNPUBLISH: {
+ recovered =
+ volumes.at(volumeId).sequence->add(std::function<Future<Nothing>()>(
+ defer(self(), &Self::nodeUnpublish, volumeId)));
+ break;
+ }
+ case csi::state::VolumeState::UNKNOWN: {
+ recovered = Failure(
+ "Volume '" + volumeId + "' is in " +
+ stringify(volumes.at(volumeId).state.state()) + " state");
}
- futures.push_back(recovered);
+ // NOTE: We avoid using a default clause for the following
+ // values in proto3's open enum to enable the compiler to detect
+ // missing enum cases for us. See:
+ // https://github.com/google/protobuf/issues/3917
+ case google::protobuf::kint32min:
+ case google::protobuf::kint32max: {
+ UNREACHABLE();
+ }
}
+
+ futures.push_back(recovered);
}
return collect(futures).then([] { return Nothing(); });
@@ -906,7 +902,7 @@ StorageLocalResourceProviderProcess::recoverResourceProviderState()
return Nothing();
}
- Result<ResourceProviderState> resourceProviderState =
+ Try<ResourceProviderState> resourceProviderState =
::protobuf::read<ResourceProviderState>(statePath);
if (resourceProviderState.isError()) {
@@ -915,28 +911,26 @@ StorageLocalResourceProviderProcess::recoverResourceProviderState()
"': " + resourceProviderState.error());
}
- if (resourceProviderState.isSome()) {
- foreach (
- Operation& operation,
- *resourceProviderState->mutable_operations()) {
- upgradeResources(operation.mutable_info());
- }
+ foreach (
+ Operation& operation,
+ *resourceProviderState->mutable_operations()) {
+ upgradeResources(operation.mutable_info());
+ }
- convertResourceFormat(
- resourceProviderState->mutable_resources(),
- POST_RESERVATION_REFINEMENT);
+ convertResourceFormat(
+ resourceProviderState->mutable_resources(),
+ POST_RESERVATION_REFINEMENT);
- foreach (const Operation& operation,
- resourceProviderState->operations()) {
- Try<id::UUID> uuid = id::UUID::fromBytes(operation.uuid().value());
+ foreach (const Operation& operation,
+ resourceProviderState->operations()) {
+ Try<id::UUID> uuid = id::UUID::fromBytes(operation.uuid().value());
- CHECK_SOME(uuid);
+ CHECK_SOME(uuid);
- operations[uuid.get()] = operation;
- }
-
- totalResources = resourceProviderState->resources();
+ operations[uuid.get()] = operation;
}
+
+ totalResources = resourceProviderState->resources();
}
return Nothing();
http://git-wip-us.apache.org/repos/asf/mesos/blob/fda054b5/src/slave/containerizer/mesos/paths.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/paths.cpp b/src/slave/containerizer/mesos/paths.cpp
index d6ea618..4cec2d2 100644
--- a/src/slave/containerizer/mesos/paths.cpp
+++ b/src/slave/containerizer/mesos/paths.cpp
@@ -276,7 +276,7 @@ Result<ContainerTermination> getContainerTermination(
return None();
}
- Result<ContainerTermination> termination =
+ Try<ContainerTermination> termination =
::protobuf::read<ContainerTermination>(path);
if (termination.isError()) {
@@ -284,10 +284,8 @@ Result<ContainerTermination> getContainerTermination(
termination.error());
}
- if (termination.isSome()) {
- convertResourceFormat(
- termination->mutable_limited_resources(), POST_RESERVATION_REFINEMENT);
- }
+ convertResourceFormat(
+ termination->mutable_limited_resources(), POST_RESERVATION_REFINEMENT);
return termination;
}
@@ -329,7 +327,7 @@ Result<ContainerConfig> getContainerConfig(
return None();
}
- Result<ContainerConfig> containerConfig =
+ Try<ContainerConfig> containerConfig =
::protobuf::read<ContainerConfig>(path);
if (containerConfig.isError()) {
@@ -337,18 +335,16 @@ Result<ContainerConfig> getContainerConfig(
containerConfig.error());
}
- if (containerConfig.isSome()) {
- convertResourceFormat(
- containerConfig->mutable_executor_info()->mutable_resources(),
- POST_RESERVATION_REFINEMENT);
+ convertResourceFormat(
+ containerConfig->mutable_executor_info()->mutable_resources(),
+ POST_RESERVATION_REFINEMENT);
- convertResourceFormat(
- containerConfig->mutable_task_info()->mutable_resources(),
- POST_RESERVATION_REFINEMENT);
+ convertResourceFormat(
+ containerConfig->mutable_task_info()->mutable_resources(),
+ POST_RESERVATION_REFINEMENT);
- convertResourceFormat(
- containerConfig->mutable_resources(), POST_RESERVATION_REFINEMENT);
- }
+ convertResourceFormat(
+ containerConfig->mutable_resources(), POST_RESERVATION_REFINEMENT);
return containerConfig;
}
@@ -439,7 +435,7 @@ Result<ContainerLaunchInfo> getContainerLaunchInfo(
return None();
}
- const Result<ContainerLaunchInfo>& containerLaunchInfo =
+ Try<ContainerLaunchInfo> containerLaunchInfo =
::protobuf::read<ContainerLaunchInfo>(path);
if (containerLaunchInfo.isError()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/fda054b5/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
index 1ab66c1..548c524 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
@@ -256,18 +256,12 @@ Future<Nothing> MetadataManagerProcess::recover()
return Nothing();
}
- Result<Images> images = ::protobuf::read<Images>(storedImagesPath);
+ Try<Images> images = ::protobuf::read<Images>(storedImagesPath);
if (images.isError()) {
return Failure("Failed to read images from '" + storedImagesPath + "' " +
images.error());
}
- if (images.isNone()) {
- // This could happen if the slave died after opening the file for
- // writing but before persisted on disk.
- return Failure("Unexpected empty images file '" + storedImagesPath + "'");
- }
-
foreach (const Image& image, images.get().images()) {
const string imageReference = stringify(image.reference());
http://git-wip-us.apache.org/repos/asf/mesos/blob/fda054b5/src/slave/containerizer/mesos/provisioner/provisioner.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/provisioner.cpp b/src/slave/containerizer/mesos/provisioner/provisioner.cpp
index 7621c49..57a437b 100644
--- a/src/slave/containerizer/mesos/provisioner/provisioner.cpp
+++ b/src/slave/containerizer/mesos/provisioner/provisioner.cpp
@@ -420,7 +420,6 @@ Future<Nothing> ProvisionerProcess::recover(
info->rootfses.put(backend, rootfses.get()[backend]);
}
- Result<ContainerLayers> layers = None();
const string path = provisioner::paths::getLayersFilePath(
rootDir, containerId);
@@ -430,19 +429,18 @@ Future<Nothing> ProvisionerProcess::recover(
VLOG(1) << "Layers path '" << path << "' is missing for container' "
<< containerId << "'";
} else {
- layers = ::protobuf::read<ContainerLayers>(path);
- }
+ Try<ContainerLayers> layers = ::protobuf::read<ContainerLayers>(path);
+ if (layers.isError()) {
+ return Failure(
+ "Failed to recover layers for container '" +
+ stringify(containerId) + "': " + layers.error());
+ }
- if (layers.isError()) {
- return Failure(
- "Failed to recover layers for container '" + stringify(containerId) +
- "': " + layers.error());
- } else if (layers.isSome()) {
info->layers = vector<string>();
std::copy(
- layers->paths().begin(),
- layers->paths().end(),
- std::back_inserter(info->layers.get()));
+ layers->paths().begin(),
+ layers->paths().end(),
+ std::back_inserter(info->layers.get()));
}
infos.put(containerId, info);
http://git-wip-us.apache.org/repos/asf/mesos/blob/fda054b5/src/slave/state.cpp
----------------------------------------------------------------------
diff --git a/src/slave/state.cpp b/src/slave/state.cpp
index 5428b34..0bc0cca 100644
--- a/src/slave/state.cpp
+++ b/src/slave/state.cpp
@@ -151,8 +151,7 @@ Try<SlaveState> SlaveState::recover(
return state;
}
- Result<SlaveInfo> slaveInfo = ::protobuf::read<SlaveInfo>(path);
-
+ Try<SlaveInfo> slaveInfo = ::protobuf::read<SlaveInfo>(path);
if (slaveInfo.isError()) {
const string& message = "Failed to read agent info from '" + path + "': " +
slaveInfo.error();
@@ -165,13 +164,6 @@ Try<SlaveState> SlaveState::recover(
}
}
- if (slaveInfo.isNone()) {
- // This could happen if the slave died after opening the file for
- // writing but before it checkpointed anything.
- LOG(WARNING) << "Found empty agent info file '" << path << "'";
- return state;
- }
-
convertResourceFormat(
slaveInfo.get().mutable_resources(),
POST_RESERVATION_REFINEMENT);
@@ -227,7 +219,7 @@ Try<FrameworkState> FrameworkState::recover(
return state;
}
- const Result<FrameworkInfo>& frameworkInfo =
+ const Try<FrameworkInfo> frameworkInfo =
::protobuf::read<FrameworkInfo>(path);
if (frameworkInfo.isError()) {
@@ -243,13 +235,6 @@ Try<FrameworkState> FrameworkState::recover(
}
}
- if (frameworkInfo.isNone()) {
- // This could happen if the slave died after opening the file for
- // writing but before it checkpointed anything.
- LOG(WARNING) << "Found empty framework info file '" << path << "'";
- return state;
- }
-
state.info = frameworkInfo.get();
// Read the framework pid.
@@ -394,8 +379,7 @@ Try<ExecutorState> ExecutorState::recover(
return state;
}
- Result<ExecutorInfo> executorInfo = ::protobuf::read<ExecutorInfo>(path);
-
+ Try<ExecutorInfo> executorInfo = ::protobuf::read<ExecutorInfo>(path);
if (executorInfo.isError()) {
message = "Failed to read executor info from '" + path + "': " +
executorInfo.error();
@@ -409,13 +393,6 @@ Try<ExecutorState> ExecutorState::recover(
}
}
- if (executorInfo.isNone()) {
- // This could happen if the slave died after opening the file for
- // writing but before it checkpointed anything.
- LOG(WARNING) << "Found empty executor info file '" << path << "'";
- return state;
- }
-
convertResourceFormat(
executorInfo.get().mutable_resources(),
POST_RESERVATION_REFINEMENT);
@@ -595,7 +572,7 @@ Try<TaskState> TaskState::recover(
return state;
}
- Result<Task> task = ::protobuf::read<Task>(path);
+ Try<Task> task = ::protobuf::read<Task>(path);
if (task.isError()) {
message = "Failed to read task info from '" + path + "': " + task.error();
@@ -609,13 +586,6 @@ Try<TaskState> TaskState::recover(
}
}
- if (task.isNone()) {
- // This could happen if the slave died after opening the file for
- // writing but before it checkpointed anything.
- LOG(WARNING) << "Found empty task info file '" << path << "'";
- return state;
- }
-
convertResourceFormat(
task.get().mutable_resources(),
POST_RESERVATION_REFINEMENT);
http://git-wip-us.apache.org/repos/asf/mesos/blob/fda054b5/src/tests/protobuf_io_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/protobuf_io_tests.cpp b/src/tests/protobuf_io_tests.cpp
index 4a2e3a3..ddbda03 100644
--- a/src/tests/protobuf_io_tests.cpp
+++ b/src/tests/protobuf_io_tests.cpp
@@ -150,15 +150,14 @@ TEST_F(ProtobufIOTest, RepeatedPtrField)
Try<Nothing> write = ::protobuf::write(file, expected);
ASSERT_SOME(write);
- Result<RepeatedPtrField<FrameworkID>> read =
+ Try<RepeatedPtrField<FrameworkID>> actual =
::protobuf::read<RepeatedPtrField<FrameworkID>>(file);
- ASSERT_SOME(read);
- RepeatedPtrField<FrameworkID> actual = read.get();
+ ASSERT_SOME(actual);
- ASSERT_EQ(expected.size(), actual.size());
+ ASSERT_EQ(expected.size(), actual->size());
for (size_t i = 0; i < size; i++) {
- EXPECT_EQ(expected.Get(i), actual.Get(i));
+ EXPECT_EQ(expected.Get(i), actual->Get(i));
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/fda054b5/src/tests/slave_recovery_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_recovery_tests.cpp b/src/tests/slave_recovery_tests.cpp
index e305d74..fc26987 100644
--- a/src/tests/slave_recovery_tests.cpp
+++ b/src/tests/slave_recovery_tests.cpp
@@ -125,10 +125,10 @@ TEST_F(SlaveStateTest, CheckpointProtobufMessage)
SlaveID expected;
expected.set_value("agent1");
- const string& file = "slave.id";
+ const string file = "slave.id";
slave::state::checkpoint(file, expected);
- const Result<SlaveID>& actual = ::protobuf::read<SlaveID>(file);
+ const Try<SlaveID> actual = ::protobuf::read<SlaveID>(file);
ASSERT_SOME(actual);
EXPECT_SOME_EQ(expected, actual);
@@ -144,7 +144,7 @@ TEST_F(SlaveStateTest, CheckpointRepeatedProtobufMessages)
const string file = "resources-file";
slave::state::checkpoint(file, expected);
- Result<RepeatedPtrField<Resource>> actual =
+ Try<RepeatedPtrField<Resource>> actual =
::protobuf::read<RepeatedPtrField<Resource>>(file);
ASSERT_SOME(actual);
[3/5] mesos git commit: Added `state::read` to complement
`state::checkpoint`.
Posted by mp...@apache.org.
Added `state::read` to complement `state::checkpoint`.
Review: https://reviews.apache.org/r/65023
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ef303905
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ef303905
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ef303905
Branch: refs/heads/master
Commit: ef303905e0be96f28d79a569afd004ec75f98296
Parents: fda054b
Author: Michael Park <mp...@apache.org>
Authored: Fri Jan 5 09:52:51 2018 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Wed Jan 10 15:57:11 2018 -0800
----------------------------------------------------------------------
src/slave/state.hpp | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ef303905/src/slave/state.hpp
----------------------------------------------------------------------
diff --git a/src/slave/state.hpp b/src/slave/state.hpp
index 01abb50..0950614 100644
--- a/src/slave/state.hpp
+++ b/src/slave/state.hpp
@@ -74,6 +74,28 @@ struct TaskState;
Try<State> recover(const std::string& rootDir, bool strict);
+// Reads the protobuf message(s) from the given path.
+// `T` may be either a single protobuf message or a sequence of messages
+// if `T` is a specialization of `google::protobuf::RepeatedPtrField`.
+template <typename T>
+Try<T> read(const std::string& path)
+{
+ Try<T> result = ::protobuf::read<T>(path);
+ if (result.isError()) {
+ return Error(result.error());
+ }
+
+ upgradeResources(&result.get());
+ return result;
+}
+
+template <>
+inline Try<std::string> read<std::string>(const std::string& path)
+{
+ return os::read(path);
+}
+
+
namespace internal {
inline Try<Nothing> checkpoint(