bmahler commented on code in PR #463:
URL: https://github.com/apache/mesos/pull/463#discussion_r1454033642


##########
src/slave/containerizer/mesos/isolators/xfs/disk.cpp:
##########
@@ -464,6 +467,38 @@ Future<Nothing> XfsDiskIsolatorProcess::recover(
     }
   }
 
+  // Assign project IDs to sandboxes that were previously not managed by this
+  // isolator. Quotas will be set later when the containerizer will send an
+  // update call upon executor re-registration.
+  foreach (const string& sandbox, unmanaged) {
+    ContainerID containerId;
+    containerId.set_value(Path(sandbox).basename());
+    CHECK(!infos.contains(containerId));
+
+    // Ignore the container if it is not alive or if it is a known orphan. In
+    // the latter case the containerizer will send a cleanup call for it 
anyway.
+    if (orphans.contains(containerId) || !alive.contains(containerId)) {
+      continue;
+    }
+
+    Option<prid_t> projectId = nextProjectId();
+    if (projectId.isNone()) {
+      return Failure("Failed to assign project ID, range exhausted");

Review Comment:
   "Failed to assign project to sandbox <sandbox>: Failed to obtain next 
project ID: range exhausted" (wrap lines at 80 though, so split onto multiple 
lines).



##########
src/slave/containerizer/mesos/isolators/xfs/disk.cpp:
##########
@@ -274,6 +274,8 @@ Future<Nothing> XfsDiskIsolatorProcess::recover(
     alive.insert(state.container_id());
   }
 
+  list<string> unmanaged;

Review Comment:
   we default to vector unless list is needed for e.g. O(1) insertion, so: 
s/list/vector/



##########
src/tests/containerizer/xfs_quota_tests.cpp:
##########
@@ -101,6 +101,21 @@ static QuotaInfo makeQuotaInfo(
 }
 
 
+static bool waitForFileCreation(
+    const string& path,
+    const Duration& duration = Seconds(60))
+{
+  Stopwatch timer;
+  timer.start();
+  while (!os::exists(path)) {
+    if ((duration > Duration::zero()) && (timer.elapsed() > duration))
+      break;
+    os::sleep(Milliseconds(50));
+  }

Review Comment:
   Looks like this can be simplified?
   
   ```
     while (!os::exists(path) && timer.elapsed() < duration) {
       os::sleep(Milliseconds(50));
     }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to