You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2019/08/08 04:54:50 UTC
[mesos] 02/06: Supported multiple quota paths in the `disk/xfs`
isolator.
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 <jp...@apache.org>
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