This is an automated email from the ASF dual-hosted git repository. jpeach pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 3a209b0a83751e500395844e1d1e87d15543b681 Author: James Peach <[email protected]> AuthorDate: Wed Aug 7 20:21:56 2019 -0700 Supported multiple quota paths in the `disk/xfs` isolator. The `disk/xfs` isolator assumed that there would only be a single directory path for each project quota. When we apply project quotas to the overlayfs upperdir, that won't be true any more, since the upperdir will come under the same quota as the task sandbox. Update the quota reclamation tracking to keep a set of disk paths that the quota has been applied to, and only reclaim the project ID once all those paths have been removed. Review: https://reviews.apache.org/r/71193/ --- .../containerizer/mesos/isolators/xfs/disk.cpp | 102 ++++++++++++--------- .../containerizer/mesos/isolators/xfs/disk.hpp | 11 ++- 2 files changed, 68 insertions(+), 45 deletions(-) diff --git a/src/slave/containerizer/mesos/isolators/xfs/disk.cpp b/src/slave/containerizer/mesos/isolators/xfs/disk.cpp index 646330c..5454432 100644 --- a/src/slave/containerizer/mesos/isolators/xfs/disk.cpp +++ b/src/slave/containerizer/mesos/isolators/xfs/disk.cpp @@ -345,23 +345,19 @@ Future<Nothing> XfsDiskIsolatorProcess::recover( // about this persistent volume. CHECK(!scheduledProjects.contains(projectId.get())) << "Duplicate project ID " << projectId.get() - << " assigned to '" << directory << "'" - << " and '" << scheduledProjects.at(projectId.get()).second << "'"; + << " assigned to '" << directory << "' and '" + << *scheduledProjects.at(projectId.get()).directories.begin() << "'"; freeProjectIds -= projectId.get(); if (totalProjectIds.contains(projectId.get())) { --metrics.project_ids_free; } - Try<string> devname = xfs::getDeviceForPath(directory); - if (devname.isError()) { + Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory); + if (scheduled.isError()) { LOG(ERROR) << "Unable to schedule project ID " << projectId.get() - << " for reclaimation: " << devname.error(); - continue; + << " for reclaimation: " << scheduled.error(); } - - scheduledProjects.put( - projectId.get(), make_pair(devname.get(), directory)); } return Nothing(); @@ -567,16 +563,10 @@ Future<Nothing> XfsDiskIsolatorProcess::update( << " for project " << projectId.get() << " to " << status->softLimit << "/" << status->hardLimit; - if (!scheduledProjects.contains(projectId.get())) { - Try<string> devname = xfs::getDeviceForPath(directory); - if (devname.isError()) { - LOG(ERROR) << "Unable to schedule project " << projectId.get() - << " for reclaimation: " << devname.error(); - continue; - } - - scheduledProjects.put( - projectId.get(), make_pair(devname.get(), directory)); + Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory); + if (scheduled.isError()) { + LOG(ERROR) << "Unable to schedule project " << projectId.get() + << " for reclaimation: " << scheduled.error(); } } @@ -732,21 +722,11 @@ Future<Nothing> XfsDiskIsolatorProcess::cleanup(const ContainerID& containerId) // we determine that the persistent volume is no longer present. foreachpair ( const string& directory, const Info::PathInfo& pathInfo, info->paths) { - // If we assigned a project ID to a persistent volume, it might - // already be scheduled for reclaimation. - if (scheduledProjects.contains(pathInfo.projectId)) { - continue; - } - - Try<string> devname = xfs::getDeviceForPath(directory); - if (devname.isError()) { + Try<Nothing> scheduled = scheduleProjectRoot(pathInfo.projectId, directory); + if (scheduled.isError()) { LOG(ERROR) << "Unable to schedule project " << pathInfo.projectId - << " for reclaimation: " << devname.error(); - continue; + << " for reclaimation: " << scheduled.error(); } - - scheduledProjects.put( - pathInfo.projectId, make_pair(devname.get(), directory)); } infos.erase(containerId); @@ -781,6 +761,33 @@ void XfsDiskIsolatorProcess::returnProjectId( } +Try<Nothing> XfsDiskIsolatorProcess::scheduleProjectRoot( + prid_t projectId, + const string& rootDir) +{ + Try<string> devname = xfs::getDeviceForPath(rootDir); + + if (devname.isError()) { + return Error(devname.error()); + } + + if (!scheduledProjects.contains(projectId)) { + scheduledProjects.put(projectId, ProjectRoots{devname.get(), {rootDir}}); + } else { + ProjectRoots& roots = scheduledProjects.at(projectId); + + if (roots.deviceName != devname.get()) { + return Error(strings::format( + "Conflicting device names '%s' and '%s' for project ID %s", + roots.deviceName, devname.get(), projectId).get()); + } + + roots.directories.insert(rootDir); + } + + return Nothing(); +} + void XfsDiskIsolatorProcess::reclaimProjectIds() { // Note that we need both the directory we assigned the project ID to, @@ -789,22 +796,29 @@ void XfsDiskIsolatorProcess::reclaimProjectIds() // need the latter to make the corresponding quota record updates. foreachpair ( - prid_t projectId, const auto& dir, utils::copy(scheduledProjects)) { - if (os::exists(dir.second)) { - continue; + prid_t projectId, auto& roots, utils::copy(scheduledProjects)) { + // Stop tracking any directories that have already been removed. + foreach (const string& directory, utils::copy(roots.directories)) { + if (!os::exists(directory)) { + roots.directories.erase(directory); + + VLOG(1) << "Droppped path '" << directory + << "' from project ID " << projectId; + } } - Try<Nothing> status = xfs::clearProjectQuota(dir.first, projectId); - if (status.isError()) { - LOG(ERROR) << "Failed to clear quota for '" - << dir.second << "': " << status.error(); - } + if (roots.directories.empty()) { + Try<Nothing> status = xfs::clearProjectQuota(roots.deviceName, projectId); + if (status.isError()) { + LOG(ERROR) << "Failed to clear quota for project ID " + << projectId << "': " << status.error(); + } - returnProjectId(projectId); - scheduledProjects.erase(projectId); + returnProjectId(projectId); + scheduledProjects.erase(projectId); - LOG(INFO) << "Reclaimed project ID " << projectId - << " from '" << dir.second << "'"; + LOG(INFO) << "Reclaimed project ID " << projectId; + } } } diff --git a/src/slave/containerizer/mesos/isolators/xfs/disk.hpp b/src/slave/containerizer/mesos/isolators/xfs/disk.hpp index 94d44e7..16bb432 100644 --- a/src/slave/containerizer/mesos/isolators/xfs/disk.hpp +++ b/src/slave/containerizer/mesos/isolators/xfs/disk.hpp @@ -27,6 +27,7 @@ #include <stout/bytes.hpp> #include <stout/duration.hpp> #include <stout/hashmap.hpp> +#include <stout/hashset.hpp> #include "slave/flags.hpp" @@ -93,6 +94,11 @@ private: process::Promise<mesos::slave::ContainerLimitation> limitation; }; + struct ProjectRoots { + std::string deviceName; + hashset<std::string> directories; + }; + XfsDiskIsolatorProcess( Duration watchInterval, xfs::QuotaPolicy quotaPolicy, @@ -113,6 +119,9 @@ private: // that are not. void reclaimProjectIds(); + Try<Nothing> scheduleProjectRoot( + prid_t projectId, const std::string& rootDir); + const Duration watchInterval; const Duration projectWatchInterval; xfs::QuotaPolicy quotaPolicy; @@ -123,7 +132,7 @@ private: // Track the device and filesystem path of unused project IDs we want // to reclaim. - hashmap<prid_t, std::pair<std::string, std::string>> scheduledProjects; + hashmap<prid_t, ProjectRoots> scheduledProjects; // Metrics used by the XFS disk isolator. struct Metrics
