Repository: mesos Updated Branches: refs/heads/master 2402f99ca -> a483fb0d0
Ensured that agent does not delete volume upon grow or shrink. Previously, `slave::syncCheckpointedResources` implementation will delete a persistent volume using `Resources::contains` check, which could cause a resized volume being deleted. The function was rewritten to compare `set_difference` between old and new paths for all persistent volumes and perform creation/deletion accordingly. Review: https://reviews.apache.org/r/66218/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/57e705a4 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/57e705a4 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/57e705a4 Branch: refs/heads/master Commit: 57e705a475ad03bfbef2605b3573a601239e6242 Parents: 2402f99 Author: Zhitao Li <zhitaoli...@gmail.com> Authored: Thu May 3 17:04:07 2018 -0700 Committer: Chun-Hung Hsiao <chhs...@mesosphere.io> Committed: Thu May 3 17:04:07 2018 -0700 ---------------------------------------------------------------------- src/slave/slave.cpp | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/57e705a4/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 6ca3d79..69280d9 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -4231,8 +4231,31 @@ void Slave::checkpointResourcesMessage( Try<Nothing> Slave::syncCheckpointedResources( const Resources& newCheckpointedResources) { - Resources oldVolumes = checkpointedResources.persistentVolumes(); - Resources newVolumes = newCheckpointedResources.persistentVolumes(); + auto toPathMap = [](const string& workDir, const Resources& resources) { + hashmap<string, Resource> pathMap; + const Resources& persistentVolumes = resources.persistentVolumes(); + + foreach (const Resource& volume, persistentVolumes) { + // This is validated in master. + CHECK(Resources::isReserved(volume)); + string path = paths::getPersistentVolumePath(workDir, volume); + pathMap[path] = volume; + } + + return pathMap; + }; + + const hashmap<string, Resource> oldPathMap = + toPathMap(flags.work_dir, checkpointedResources); + + const hashmap<string, Resource> newPathMap = + toPathMap(flags.work_dir, newCheckpointedResources); + + const hashset<string> oldPaths = oldPathMap.keys(); + const hashset<string> newPaths = newPathMap.keys(); + + const hashset<string> createPaths = newPaths - oldPaths; + const hashset<string> deletePaths = oldPaths - newPaths; // Create persistent volumes that do not already exist. // @@ -4240,15 +4263,8 @@ Try<Nothing> Slave::syncCheckpointedResources( // to support multiple disks, or raw disks. Depending on the // DiskInfo, we may want to create either directories under a root // directory, or LVM volumes from a given device. - foreach (const Resource& volume, newVolumes) { - // This is validated in master. - CHECK(Resources::isReserved(volume)); - - if (oldVolumes.contains(volume)) { - continue; - } - - string path = paths::getPersistentVolumePath(flags.work_dir, volume); + foreach (const string& path, createPaths) { + const Resource& volume = newPathMap.at(path); // If creation of persistent volume fails, the agent exits. string volumeDescription = "persistent volume " + @@ -4278,12 +4294,8 @@ Try<Nothing> Slave::syncCheckpointedResources( // remove the filesystem objects for the removed volume. Note that // for MOUNT disks, we don't remove the root directory (mount point) // of the volume. - foreach (const Resource& volume, oldVolumes) { - if (newVolumes.contains(volume)) { - continue; - } - - string path = paths::getPersistentVolumePath(flags.work_dir, volume); + foreach (const string& path, deletePaths) { + const Resource& volume = oldPathMap.at(path); LOG(INFO) << "Deleting persistent volume '" << volume.disk().persistence().id()
