mesos git commit: Fixed flakiness in MasterAllocatorTest.ResourcesUnused.

[neilc]

Repository: mesos
Updated Branches:
  refs/heads/master d225d4d41 -> 6fe4be921


Fixed flakiness in MasterAllocatorTest.ResourcesUnused.

In the test, `sched1` registers with the master and launches a task on
half the resources it is offered. Then `sched2` registers and should be
offered the remaining resources. The problem is that, because the test
did not pause the clock, a batch allocation might occur between the
first task launch and the second scheduler registering. The test tried
to account for this (by having `sched1` decline any additional offers it
receives), but this was not done correctly: declining the offer will
result in calling `recoverResources` again, which the test did not
account for.

Seems simpler to instead pause the clock, which should prevent the
possibility of a batch allocation in the first place.

Review: https://reviews.apache.org/r/59509


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6fe4be92
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6fe4be92
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6fe4be92

Branch: refs/heads/master
Commit: 6fe4be92180d125753729f26887aa9fe853203a7
Parents: d225d4d
Author: Neil Conway 
Authored: Tue May 23 16:37:36 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 09:54:55 2017 -0700

--
 src/tests/master_allocator_tests.cpp | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/6fe4be92/src/tests/master_allocator_tests.cpp
--
diff --git a/src/tests/master_allocator_tests.cpp 
b/src/tests/master_allocator_tests.cpp
index 49d87af..750e030 100644
--- a/src/tests/master_allocator_tests.cpp
+++ b/src/tests/master_allocator_tests.cpp
@@ -208,6 +208,8 @@ TYPED_TEST(MasterAllocatorTest, SingleFramework)
 // reoffered appropriately.
 TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
 {
+  Clock::pause();
+
   TestAllocator allocator;
 
   EXPECT_CALL(allocator, initialize(_, _, _, _));
@@ -221,7 +223,10 @@ TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
   slave::Flags slaveFlags = this->CreateSlaveFlags();
   slaveFlags.resources = Some("cpus:2;mem:1024");
 
-  EXPECT_CALL(allocator, addSlave(_, _, _, _, _, _));
+  Future addSlave;
+  EXPECT_CALL(allocator, addSlave(_, _, _, _, _, _))
+.WillOnce(DoAll(InvokeAddSlave(),
+FutureSatisfy()));
 
   Owned detector = master.get()->createDetector();
 
@@ -229,6 +234,11 @@ TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
 this->StartSlave(detector.get(), , slaveFlags);
   ASSERT_SOME(slave);
 
+  Clock::advance(slaveFlags.authentication_backoff_factor);
+  Clock::advance(slaveFlags.registration_backoff_factor);
+
+  AWAIT_READY(addSlave);
+
   MockScheduler sched1;
   MesosSchedulerDriver driver1(
   , DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
@@ -237,13 +247,6 @@ TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
 
   EXPECT_CALL(sched1, registered(_, _, _));
 
-  // We decline offers that we aren't expecting so that the resources
-  // get aggregated. Note that we need to do this _first_ and
-  // _separate_ from the expectation below so that this expectation is
-  // checked last and matches all possible offers.
-  EXPECT_CALL(sched1, resourceOffers(_, _))
-.WillRepeatedly(DeclineOffers());
-
   // The first offer will contain all of the slave's resources, since
   // this is the only framework running so far. Launch a task that
   // uses less than that to leave some resources unused.
@@ -267,7 +270,8 @@ TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
 
   // We need to wait until the allocator knows about the unused
   // resources to start the second framework so that we get the
-  // expected offer.
+  // expected offer. Because the clock is paused, a batch allocation
+  // will not occur in the time before the second framework registers.
   AWAIT_READY(recoverResources);
 
   FrameworkInfo frameworkInfo2 = DEFAULT_FRAMEWORK_INFO;

[03/10] mesos git commit: Implemented passing Image::Secret Puller::pull().

[gilbert]

Implemented passing Image::Secret Puller::pull().

Review: https://reviews.apache.org/r/59013


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7ca46e24
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7ca46e24
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7ca46e24

Branch: refs/heads/master
Commit: 7ca46e24b3339ba27e88b99ea95362c956ef03c1
Parents: 6b83541
Author: Gilbert Song 
Authored: Wed May 3 14:14:31 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 .../mesos/provisioner/docker/local_puller.cpp  |  3 ++-
 .../mesos/provisioner/docker/local_puller.hpp  |  3 ++-
 .../mesos/provisioner/docker/puller.hpp|  3 ++-
 .../mesos/provisioner/docker/registry_puller.cpp   | 12 
 .../mesos/provisioner/docker/registry_puller.hpp   |  3 ++-
 .../mesos/provisioner/docker/store.cpp | 17 +++--
 .../containerizer/provisioner_docker_tests.cpp | 12 +++-
 7 files changed, 38 insertions(+), 15 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/7ca46e24/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
index 5bc68d2..c4879ec 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
@@ -122,7 +122,8 @@ LocalPuller::~LocalPuller()
 Future LocalPuller::pull(
 const spec::ImageReference& reference,
 const string& directory,
-const string& backend)
+const string& backend,
+const Option& config)
 {
   return dispatch(
   process.get(),

http://git-wip-us.apache.org/repos/asf/mesos/blob/7ca46e24/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
b/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
index 344baab..4d2e497 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
@@ -49,7 +49,8 @@ public:
   process::Future pull(
   const ::docker::spec::ImageReference& reference,
   const std::string& directory,
-  const std::string& backend);
+  const std::string& backend,
+  const Option& config = None());
 
 private:
   explicit LocalPuller(process::Owned _process);

http://git-wip-us.apache.org/repos/asf/mesos/blob/7ca46e24/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
b/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
index 5ff1846..a79f2ad 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
@@ -61,7 +61,8 @@ public:
   virtual process::Future pull(
   const ::docker::spec::ImageReference& reference,
   const std::string& directory,
-  const std::string& backend) = 0;
+  const std::string& backend,
+  const Option& config = None()) = 0;
 };
 
 } // namespace docker {

http://git-wip-us.apache.org/repos/asf/mesos/blob/7ca46e24/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
--
diff --git 
a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index f8c31ae..1a724f2 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -70,7 +70,8 @@ public:
   Future pull(
   const spec::ImageReference& reference,
   const string& directory,
-  const string& backend);
+  const string& backend,
+  const Option& config);
 
 private:
   Future _pull(
@@ -148,14 +149,16 @@ RegistryPuller::~RegistryPuller()
 Future RegistryPuller::pull(
 const spec::ImageReference& reference,
 const string& directory,
-const string& backend)
+const string& backend,
+const Option& config)
 {
   return dispatch(
   process.get(),
   ::pull,
   reference,
   directory,
-  backend);
+  backend,
+  config);
 }
 
 
@@ -209,7 +212,8 @@ static spec::ImageReference normalize(
 

[07/10] mesos git commit: Implemented passing the secret resolver to registry puller.

[gilbert]

Implemented passing the secret resolver to registry puller.

Review: https://reviews.apache.org/r/59012


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6b835417
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6b835417
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6b835417

Branch: refs/heads/master
Commit: 6b83541735deda4356bd4cb2773b2557495d8813
Parents: 32dc11a
Author: Gilbert Song 
Authored: Mon May 1 16:37:55 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 src/slave/containerizer/mesos/containerizer.cpp   |  4 +++-
 .../mesos/provisioner/appc/store.cpp  | 10 +++---
 .../mesos/provisioner/appc/store.hpp  |  6 +-
 .../mesos/provisioner/docker/puller.cpp   |  9 +++--
 .../mesos/provisioner/docker/puller.hpp   |  5 -
 .../mesos/provisioner/docker/registry_puller.cpp  | 18 +-
 .../mesos/provisioner/docker/registry_puller.hpp  |  5 -
 .../mesos/provisioner/docker/store.cpp| 14 ++
 .../mesos/provisioner/docker/store.hpp|  6 +-
 .../mesos/provisioner/provisioner.cpp | 10 --
 .../mesos/provisioner/provisioner.hpp |  6 +-
 .../containerizer/mesos/provisioner/store.cpp | 12 +---
 .../containerizer/mesos/provisioner/store.hpp |  5 -
 13 files changed, 84 insertions(+), 26 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/6b835417/src/slave/containerizer/mesos/containerizer.cpp
--
diff --git a/src/slave/containerizer/mesos/containerizer.cpp 
b/src/slave/containerizer/mesos/containerizer.cpp
index 403faa3..199202a 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -258,7 +258,9 @@ Try MesosContainerizer::create(
 return Error("Failed to create launcher: " + launcher.error());
   }
 
-  Try _provisioner = Provisioner::create(flags_);
+  Try _provisioner =
+Provisioner::create(flags_, secretResolver);
+
   if (_provisioner.isError()) {
 return Error("Failed to create provisioner: " + _provisioner.error());
   }

http://git-wip-us.apache.org/repos/asf/mesos/blob/6b835417/src/slave/containerizer/mesos/provisioner/appc/store.cpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/appc/store.cpp 
b/src/slave/containerizer/mesos/provisioner/appc/store.cpp
index dc547dd..9e65990 100644
--- a/src/slave/containerizer/mesos/provisioner/appc/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/appc/store.cpp
@@ -18,6 +18,10 @@
 
 #include 
 
+#include 
+
+#include 
+
 #include 
 #include 
 #include 
@@ -28,8 +32,6 @@
 #include 
 #include 
 
-#include 
-
 #include "slave/containerizer/mesos/provisioner/appc/cache.hpp"
 #include "slave/containerizer/mesos/provisioner/appc/fetcher.hpp"
 #include "slave/containerizer/mesos/provisioner/appc/paths.hpp"
@@ -96,7 +98,9 @@ private:
 };
 
 
-Try Store::create(const Flags& flags)
+Try Store::create(
+const Flags& flags,
+SecretResolver* secretResolver)
 {
   Try mkdir = os::mkdir(paths::getImagesDir(flags.appc_store_dir));
   if (mkdir.isError()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/6b835417/src/slave/containerizer/mesos/provisioner/appc/store.hpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/appc/store.hpp 
b/src/slave/containerizer/mesos/provisioner/appc/store.hpp
index 15c79e9..37ef779 100644
--- a/src/slave/containerizer/mesos/provisioner/appc/store.hpp
+++ b/src/slave/containerizer/mesos/provisioner/appc/store.hpp
@@ -17,6 +17,8 @@
 #ifndef __PROVISIONER_APPC_STORE_HPP__
 #define __PROVISIONER_APPC_STORE_HPP__
 
+#include 
+
 #include "slave/containerizer/mesos/provisioner/store.hpp"
 
 namespace mesos {
@@ -31,7 +33,9 @@ class StoreProcess;
 class Store : public slave::Store
 {
 public:
-  static Try create(const Flags& flags);
+  static Try create(
+  const Flags& flags,
+  SecretResolver* secretResolver = nullptr);
 
   ~Store();
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/6b835417/src/slave/containerizer/mesos/provisioner/docker/puller.cpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
b/src/slave/containerizer/mesos/provisioner/docker/puller.cpp
index ac9dae8..d7d8987 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/puller.cpp
+++ 

[02/10] mesos git commit: Fixed the comment style issue in docker/spec.hpp.

[gilbert]

Fixed the comment style issue in docker/spec.hpp.

Review: https://reviews.apache.org/r/59016


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/efbd436f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/efbd436f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/efbd436f

Branch: refs/heads/master
Commit: efbd436f3ede3bf5ec9e544c40a7a3d3f8151c3c
Parents: 92f16fc
Author: Gilbert Song 
Authored: Wed May 3 16:45:57 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 include/mesos/docker/spec.hpp | 77 +-
 1 file changed, 27 insertions(+), 50 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/efbd436f/include/mesos/docker/spec.hpp
--
diff --git a/include/mesos/docker/spec.hpp b/include/mesos/docker/spec.hpp
index b90f731..5ae6345 100644
--- a/include/mesos/docker/spec.hpp
+++ b/include/mesos/docker/spec.hpp
@@ -42,75 +42,58 @@ constexpr char WHITEOUT_PREFIX[] = ".wh.";
 constexpr char WHITEOUT_OPAQUE_PREFIX[] = ".wh..wh..opq";
 
 
-/**
- * Parse the docker image reference. Docker expects the image
- * reference to be in the following format:
- *   [REGISTRY_HOST[:REGISTRY_PORT]/]REPOSITORY[:TAG|@TYPE:DIGEST]
- *
- * This format is inherently ambiguous when dealing with repository
- * names that include forward slashes. To disambiguate, the docker
- * code looks for '.', or ':', or 'localhost' to decide if the first
- * component is a registry or a repository name. For more detail,
- * drill into the implementation of docker pull.
- *
- * See docker implementation:
- * https://github.com/docker/distribution/blob/master/reference/reference.go
- */
+// Parse the docker image reference. Docker expects the image
+// reference to be in the following format:
+//   [REGISTRY_HOST[:REGISTRY_PORT]/]REPOSITORY[:TAG|@TYPE:DIGEST]
+//
+// This format is inherently ambiguous when dealing with repository
+// names that include forward slashes. To disambiguate, the docker
+// code looks for '.', or ':', or 'localhost' to decide if the first
+// component is a registry or a repository name. For more detail,
+// drill into the implementation of docker pull.
+//
+// See docker implementation:
+// https://github.com/docker/distribution/blob/master/reference/reference.go
 Try parseImageReference(const std::string& s);
 
 
 std::ostream& operator<<(std::ostream& stream, const ImageReference& 
reference);
 
 
-/**
- * Returns the port of a docker registry.
- */
+// Returns the port of a docker registry.
 Result getRegistryPort(const std::string& registry);
 
 
-/**
- * Returns the scheme of a docker registry.
- */
+// Returns the scheme of a docker registry.
 Try getRegistryScheme(const std::string& registry);
 
 
-/**
- * Returns the host of a docker registry.
- */
+// Returns the host of a docker registry.
 std::string getRegistryHost(const std::string& registry);
 
 
-/**
- * Returns the hashmap by
- * parsing the docker config file.
- */
+// Returns the hashmap by
+// parsing the docker config file.
 Try> parseAuthConfig(
 const JSON::Object& _config);
 
-/**
- * Find the host from a docker config auth url.
- */
+
+// Find the host from a docker config auth url.
 std::string parseAuthUrl(const std::string& _url);
 
 
 namespace v1 {
 
-/**
- * Validates if the specified docker v1 image manifest conforms to the
- * Docker v1 spec. Returns the error if the validation fails.
- */
+// Validates if the specified docker v1 image manifest conforms to the
+// Docker v1 spec. Returns the error if the validation fails.
 Option validate(const ImageManifest& manifest);
 
 
-/**
- * Returns the docker v1 image manifest from the given JSON object.
- */
+// Returns the docker v1 image manifest from the given JSON object.
 Try parse(const JSON::Object& json);
 
 
-/**
- * Returns the docker v1 image manifest from the given string.
- */
+// Returns the docker v1 image manifest from the given string.
 Try parse(const std::string& s);
 
 } // namespace v1 {
@@ -118,22 +101,16 @@ Try parse(const std::string& s);
 
 namespace v2 {
 
-/**
- * Validates if the specified v2 image manifest conforms to the Docker
- * v2 spec. Returns the error if the validation fails.
- */
+// Validates if the specified v2 image manifest conforms to the Docker
+// v2 spec. Returns the error if the validation fails.
 Option validate(const ImageManifest& manifest);
 
 
-/**
- * Returns the docker v2 image manifest from the given JSON object.
- */
+// Returns the docker v2 image manifest from the given JSON object.
 Try parse(const JSON::Object& 

[04/10] mesos git commit: Implemented passing docker config depended methods.

[gilbert]

Implemented passing docker config depended methods.

Review: https://reviews.apache.org/r/59015


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/618a101c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/618a101c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/618a101c

Branch: refs/heads/master
Commit: 618a101ca8d6d3211a280c9d0ebb062835296ff0
Parents: 429eca0
Author: Gilbert Song 
Authored: Wed May 3 16:40:09 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 .../mesos/provisioner/docker/registry_puller.cpp| 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/618a101c/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
--
diff --git 
a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index f14b4a5..a5f299a 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -83,7 +83,8 @@ private:
   Future __pull(
   const spec::ImageReference& reference,
   const string& directory,
-  const string& backend);
+  const string& backend,
+  const Option& config);
 
   Future ___pull(
 const spec::ImageReference& reference,
@@ -96,7 +97,8 @@ private:
 const spec::ImageReference& reference,
 const string& directory,
 const spec::v2::ImageManifest& manifest,
-const string& backend);
+const string& backend,
+const Option& config);
 
   RegistryPullerProcess(const RegistryPullerProcess&) = delete;
   RegistryPullerProcess& operator=(const RegistryPullerProcess&) = delete;
@@ -287,14 +289,15 @@ Future RegistryPullerProcess::_pull(
   << "' to '" << directory << "'";
 
   return fetcher->fetch(manifestUri, directory)
-.then(defer(self(), ::__pull, reference, directory, backend));
+.then(defer(self(), ::__pull, reference, directory, backend, config));
 }
 
 
 Future RegistryPullerProcess::__pull(
 const spec::ImageReference& reference,
 const string& directory,
-const string& backend)
+const string& backend,
+const Option& config)
 {
   Try _manifest = os::read(path::join(directory, "manifest"));
   if (_manifest.isError()) {
@@ -315,7 +318,7 @@ Future RegistryPullerProcess::__pull(
 return Failure("'fsLayers' and 'history' have different size in manifest");
   }
 
-  return fetchBlobs(reference, directory, manifest.get(), backend)
+  return fetchBlobs(reference, directory, manifest.get(), backend, config)
 .then(defer(self(),
 ::___pull,
 reference,
@@ -430,7 +433,8 @@ Future RegistryPullerProcess::fetchBlobs(
 const spec::ImageReference& reference,
 const string& directory,
 const spec::v2::ImageManifest& manifest,
-const string& backend)
+const string& backend,
+const Option& config)
 {
   // First, find all the blobs that need to be fetched.
   //

[09/10] mesos git commit: Supported Image::Secret in docker URI fetcher plugin.

[gilbert]

Supported Image::Secret in docker URI fetcher plugin.

Review: https://reviews.apache.org/r/59018


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4faca73e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4faca73e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4faca73e

Branch: refs/heads/master
Commit: 4faca73e9608664a5fcf46cc95951c318a64861f
Parents: 4932045
Author: Gilbert Song 
Authored: Mon May 22 20:48:26 2017 +0800
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 src/uri/fetchers/docker.cpp | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/4faca73e/src/uri/fetchers/docker.cpp
--
diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp
index 6436f74..7f3e4e7 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -499,13 +499,15 @@ Future DockerFetcherPlugin::fetch(
   process.get(),
   ::fetch,
   uri,
-  directory);
+  directory,
+  data);
 }
 
 
 Future DockerFetcherPluginProcess::fetch(
 const URI& uri,
-const string& directory)
+const string& directory,
+const Option& data)
 {
   // TODO(gilbert): Convert the `uri` to ::docker::spec::ImageReference
   // and pass it all the way down to avoid the complicated URI conversion
@@ -531,8 +533,25 @@ Future DockerFetcherPluginProcess::fetch(
 directory + "': " + mkdir.error());
   }
 
+  hashmap _auths;
+
+  // 'data' is expected as a docker config in JSON format.
+  if (data.isSome()) {
+Try> secretAuths =
+  spec::parseAuthConfig(data.get());
+
+if (secretAuths.isError()) {
+  return Failure("Failed to parse docker config: " + secretAuths.error());
+}
+
+_auths = secretAuths.get();
+  }
+
+  // The 'secretAuths' takes the precedence over the default auths.
+  _auths.insert(auths.begin(), auths.end());
+
   // Use the 'Basic' credential to pull the manifest/blob by default.
-  http::Headers basicAuthHeaders = getAuthHeaderBasic(uri, auths);
+  http::Headers basicAuthHeaders = getAuthHeaderBasic(uri, _auths);
 
   if (uri.scheme() == "docker-blob") {
 return fetchBlob(uri, directory, basicAuthHeaders);

[08/10] mesos git commit: Added support for docker spec helper 'parseAuthConfig()'.

[gilbert]

Added support for docker spec helper 'parseAuthConfig()'.

Review: https://reviews.apache.org/r/59017


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4932045b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4932045b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4932045b

Branch: refs/heads/master
Commit: 4932045ba723debb2573eda76160d9abaadd7534
Parents: efbd436
Author: Gilbert Song 
Authored: Wed May 3 17:40:20 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 include/mesos/docker/spec.hpp |  7 ++-
 src/docker/spec.cpp   | 11 
 src/tests/containerizer/docker_spec_tests.cpp | 70 ++
 3 files changed, 50 insertions(+), 38 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/4932045b/include/mesos/docker/spec.hpp
--
diff --git a/include/mesos/docker/spec.hpp b/include/mesos/docker/spec.hpp
index 5ae6345..2879414 100644
--- a/include/mesos/docker/spec.hpp
+++ b/include/mesos/docker/spec.hpp
@@ -73,11 +73,16 @@ std::string getRegistryHost(const std::string& registry);
 
 
 // Returns the hashmap by
-// parsing the docker config file.
+// parsing the docker config file from a JSON object.
 Try> parseAuthConfig(
 const JSON::Object& _config);
 
 
+// Returns the hashmap by
+// parsing the docker config file from a string.
+Try> parseAuthConfig(const std::string& s);
+
+
 // Find the host from a docker config auth url.
 std::string parseAuthUrl(const std::string& _url);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/4932045b/src/docker/spec.cpp
--
diff --git a/src/docker/spec.cpp b/src/docker/spec.cpp
index 6b5588e..538cf18 100644
--- a/src/docker/spec.cpp
+++ b/src/docker/spec.cpp
@@ -197,6 +197,17 @@ Try> parseAuthConfig(
 }
 
 
+Try> parseAuthConfig(const string& s)
+{
+  Try json = JSON::parse(s);
+  if (json.isError()) {
+return Error("JSON parse failed: " + json.error());
+  }
+
+  return parseAuthConfig(json.get());
+}
+
+
 string parseAuthUrl(const string& _url)
 {
   string url = _url;

http://git-wip-us.apache.org/repos/asf/mesos/blob/4932045b/src/tests/containerizer/docker_spec_tests.cpp
--
diff --git a/src/tests/containerizer/docker_spec_tests.cpp 
b/src/tests/containerizer/docker_spec_tests.cpp
index d812592..5fde49a 100644
--- a/src/tests/containerizer/docker_spec_tests.cpp
+++ b/src/tests/containerizer/docker_spec_tests.cpp
@@ -199,28 +199,26 @@ TEST_F(DockerSpecTest, GetRegistrySpec)
 // for new docker config file format (e.g., ~/.docker/config.json).
 TEST_F(DockerSpecTest, ParseDockerConfig)
 {
-  Try config = JSON::parse(
-  R"~(
-  {
-"auths": {
-  "https://index.docker.io/v1/": {
-"auth": "bWVzb3M6dGVzdA==",
-"email": "u...@example.com"
-  },
-  "localhost:5000": {
-"auth": "dW5pZmllZDpjb250YWluZXJpemVy",
-"email": "u...@example.com"
-  }
+  string config =
+R"~(
+{
+  "auths": {
+"https://index.docker.io/v1/": {
+  "auth": "bWVzb3M6dGVzdA==",
+  "email": "u...@example.com"
 },
-"HttpHeaders": {
-  "User-Agent": "Docker-Client/1.10.2 (linux)"
+"localhost:5000": {
+  "auth": "dW5pZmllZDpjb250YWluZXJpemVy",
+  "email": "u...@example.com"
 }
-  })~");
-
-  ASSERT_SOME(config);
+  },
+  "HttpHeaders": {
+"User-Agent": "Docker-Client/1.10.2 (linux)"
+  }
+})~";
 
   Try> map =
-spec::parseAuthConfig(config.get());
+spec::parseAuthConfig(config);
 
   EXPECT_EQ("bWVzb3M6dGVzdA==",
 map.get()["https://index.docker.io/v1/"].auth());
@@ -237,27 +235,25 @@ TEST_F(DockerSpecTest, ParseDockerConfig)
 // for old docker config file format (e.g., ~/.dockercfg).
 TEST_F(DockerSpecTest, ParseDockercfg)
 {
-  Try dockercfg = JSON::parse(
-  R"~(
-  {
-"quay.io": {
-  "auth": "cXVheTp0ZXN0",
-  "email": "u...@example.com"
-},
-"https://index.docker.io/v1/": {
-  "auth": "cHJpdmF0ZTpyZWdpc3RyeQ==",
-  "email": "u...@example.com"
-},
-"https://192.168.0.1:5050": {
-  "auth": "aXA6YWRkcmVzcw==",
-  "email": "u...@example.com"
-}
-  })~");
-
-  

[06/10] mesos git commit: Implemented resolving an image secret in registry puller.

[gilbert]

Implemented resolving an image secret in registry puller.

Review: https://reviews.apache.org/r/59014


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/429eca05
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/429eca05
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/429eca05

Branch: refs/heads/master
Commit: 429eca054aed720b14ace059de1df3c7bcc774cf
Parents: 7ca46e2
Author: Gilbert Song 
Authored: Wed May 3 15:46:59 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 .../provisioner/docker/registry_puller.cpp  | 38 
 1 file changed, 32 insertions(+), 6 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/429eca05/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
--
diff --git 
a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index 1a724f2..f14b4a5 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -77,9 +77,15 @@ private:
   Future _pull(
   const spec::ImageReference& reference,
   const string& directory,
-  const string& backend);
+  const string& backend,
+  const Option& config = None());
 
   Future __pull(
+  const spec::ImageReference& reference,
+  const string& directory,
+  const string& backend);
+
+  Future ___pull(
 const spec::ImageReference& reference,
 const string& directory,
 const spec::v2::ImageManifest& manifest,
@@ -210,11 +216,31 @@ static spec::ImageReference normalize(
 
 
 Future RegistryPullerProcess::pull(
-const spec::ImageReference& _reference,
+const spec::ImageReference& reference,
 const string& directory,
 const string& backend,
 const Option& config)
 {
+  if (config.isNone()) {
+return _pull(reference, directory, backend);
+  }
+
+  return secretResolver->resolve(config.get())
+.then(defer(self(),
+::_pull,
+reference,
+directory,
+backend,
+lambda::_1));
+}
+
+
+Future RegistryPullerProcess::_pull(
+const spec::ImageReference& _reference,
+const string& directory,
+const string& backend,
+const Option& config)
+{
   spec::ImageReference reference = normalize(_reference, defaultRegistryUrl);
 
   URI manifestUri;
@@ -261,11 +287,11 @@ Future RegistryPullerProcess::pull(
   << "' to '" << directory << "'";
 
   return fetcher->fetch(manifestUri, directory)
-.then(defer(self(), ::_pull, reference, directory, backend));
+.then(defer(self(), ::__pull, reference, directory, backend));
 }
 
 
-Future RegistryPullerProcess::_pull(
+Future RegistryPullerProcess::__pull(
 const spec::ImageReference& reference,
 const string& directory,
 const string& backend)
@@ -291,7 +317,7 @@ Future RegistryPullerProcess::_pull(
 
   return fetchBlobs(reference, directory, manifest.get(), backend)
 .then(defer(self(),
-::__pull,
+::___pull,
 reference,
 directory,
 manifest.get(),
@@ -300,7 +326,7 @@ Future RegistryPullerProcess::_pull(
 }
 
 
-Future RegistryPullerProcess::__pull(
+Future RegistryPullerProcess::___pull(
 const spec::ImageReference& reference,
 const string& directory,
 const spec::v2::ImageManifest& manifest,

mesos git commit: Renamed RegisterAgent.agent to RegisterAgent.agents in acls.proto.

[neilc]

Repository: mesos
Updated Branches:
  refs/heads/master fe7ca914d -> d225d4d41


Renamed RegisterAgent.agent to RegisterAgent.agents in acls.proto.

Renames the field `RegisterAgent.agent` to `RegisterAgent.agents` in
order to come make it consistent with other ACLs.

Review: https://reviews.apache.org/r/59453/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d225d4d4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d225d4d4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d225d4d4

Branch: refs/heads/master
Commit: d225d4d4122e773e2416ba0d0eee653da8ced352
Parents: fe7ca91
Author: Alexander Rojas 
Authored: Wed May 24 09:24:20 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 09:43:07 2017 -0700

--
 include/mesos/authorizer/acls.proto  | 2 +-
 src/authorizer/local/authorizer.cpp  | 6 +++---
 src/tests/authorization_tests.cpp| 6 +++---
 src/tests/master_authorization_tests.cpp | 8 
 src/tests/script.cpp | 2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/d225d4d4/include/mesos/authorizer/acls.proto
--
diff --git a/include/mesos/authorizer/acls.proto 
b/include/mesos/authorizer/acls.proto
index ae0b1ea..36b3ac2 100644
--- a/include/mesos/authorizer/acls.proto
+++ b/include/mesos/authorizer/acls.proto
@@ -351,7 +351,7 @@ message ACL {
 
 // Objects: Given implicitly. Use Entity type ANY or NONE to allow or deny
 // access.
-required Entity agent = 2;
+required Entity agents = 2;
   }
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/d225d4d4/src/authorizer/local/authorizer.cpp
--
diff --git a/src/authorizer/local/authorizer.cpp 
b/src/authorizer/local/authorizer.cpp
index 89aaf4b..1f2a990 100644
--- a/src/authorizer/local/authorizer.cpp
+++ b/src/authorizer/local/authorizer.cpp
@@ -1242,7 +1242,7 @@ private:
 foreach (const ACL::RegisterAgent& acl, acls.register_agents()) {
   GenericACL acl_;
   acl_.subjects = acl.principals();
-  acl_.objects = acl.agent();
+  acl_.objects = acl.agents();
 
   acls_.push_back(acl_);
 }
@@ -1337,9 +1337,9 @@ Option LocalAuthorizer::validate(const ACLs& acls)
   }
 
   foreach (const ACL::RegisterAgent& acl, acls.register_agents()) {
-if (acl.agent().type() == ACL::Entity::SOME) {
+if (acl.agents().type() == ACL::Entity::SOME) {
   return Error(
-  "acls.register_agents.agent type must be either NONE or ANY");
+  "acls.register_agents type must be either NONE or ANY");
 }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/d225d4d4/src/tests/authorization_tests.cpp
--
diff --git a/src/tests/authorization_tests.cpp 
b/src/tests/authorization_tests.cpp
index 32aa6ac..6d85a54 100644
--- a/src/tests/authorization_tests.cpp
+++ b/src/tests/authorization_tests.cpp
@@ -4898,14 +4898,14 @@ TYPED_TEST(AuthorizationTest, RegisterAgent)
 // "foo" principal can register as an agent.
 mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
 acl->mutable_principals()->add_values("foo");
-acl->mutable_agent()->set_type(mesos::ACL::Entity::ANY);
+acl->mutable_agents()->set_type(mesos::ACL::Entity::ANY);
   }
 
   {
 // Nobody else can register as an agent.
 mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
 acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
-acl->mutable_agent()->set_type(mesos::ACL::Entity::NONE);
+acl->mutable_agents()->set_type(mesos::ACL::Entity::NONE);
   }
 
   Try create = TypeParam::create(parameterize(acls));
@@ -4936,7 +4936,7 @@ TYPED_TEST(AuthorizationTest, RegisterAgent)
 
 mesos::ACL::RegisterAgent* acl = invalid.add_register_agents();
 acl->mutable_principals()->add_values("foo");
-acl->mutable_agent()->add_values("yoda");
+acl->mutable_agents()->add_values("yoda");
 
 Try create = TypeParam::create(parameterize(invalid));
 EXPECT_ERROR(create);

http://git-wip-us.apache.org/repos/asf/mesos/blob/d225d4d4/src/tests/master_authorization_tests.cpp
--
diff --git a/src/tests/master_authorization_tests.cpp 
b/src/tests/master_authorization_tests.cpp
index e4233c1..0a2f31b 100644
--- a/src/tests/master_authorization_tests.cpp
+++ b/src/tests/master_authorization_tests.cpp
@@ -2339,7 +2339,7 @@ TEST_F(MasterAuthorizationTest, 
AuthorizedToRegisterAndReregisterAgent)
   ACLs acls;
   

mesos git commit: Added Gilbert Song to the list of committers.

[gilbert]

Repository: mesos
Updated Branches:
  refs/heads/master 2c2796a81 -> fe7ca914d


Added Gilbert Song to the list of committers.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fe7ca914
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fe7ca914
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fe7ca914

Branch: refs/heads/master
Commit: fe7ca914d6c5a645baf60ea481fafdb4bf6394cc
Parents: 2c2796a
Author: Gilbert Song 
Authored: Wed May 24 23:58:24 2017 +0800
Committer: Gilbert Song 
Committed: Wed May 24 23:58:24 2017 +0800

--
 docs/committers.md | 7 +++
 1 file changed, 7 insertions(+)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/fe7ca914/docs/committers.md
--
diff --git a/docs/committers.md b/docs/committers.md
index 5eae1be..cb4e5d3 100644
--- a/docs/committers.md
+++ b/docs/committers.md
@@ -223,6 +223,13 @@ We'd like to thank the following committers to the Apache 
Mesos project who have
   jo...@apache.org
 
 
+  -8
+  Gilbert Song
+  Mesosphere
+  
+  gilb...@apache.org
+
+
   -5
   Timothy St Clair
   Redhat

[01/10] mesos git commit: Updated protobuf comments for Image::Secret.

[gilbert]

Repository: mesos
Updated Branches:
  refs/heads/master 6fe4be921 -> 4faca73e9


Updated protobuf comments for Image::Secret.

Review: https://reviews.apache.org/r/59010


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/90385489
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/90385489
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/90385489

Branch: refs/heads/master
Commit: 903854890b86288fab88b77c6264ea6e2abb17ee
Parents: 6fe4be9
Author: Gilbert Song 
Authored: Thu May 4 16:12:09 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:29 2017 +0800

--
 include/mesos/mesos.proto| 14 ++
 include/mesos/v1/mesos.proto | 14 ++
 2 files changed, 12 insertions(+), 16 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/90385489/include/mesos/mesos.proto
--
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 5ac35f4..b063dda 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -2150,16 +2150,14 @@ message Image {
 // NOTE: This is not encrypted, therefore framework and operators
 // should enable SSL when passing this information.
 //
-// This is field is never used in Mesos before and is deprecated
-// since Mesos 1.3. Please use the `Secret::Value` field `config`
-// below (see MESOS-7088 for details).
+// This field has never been used in Mesos before and is
+// deprecated since Mesos 1.3. Please use `config` below
+// (see MESOS-7088 for details).
 optional Credential credential = 2 [deprecated = true]; // Since 1.3.
 
-// The UTF-8 character encoded byte data, which is expected as
-// a docker config file in JSON format. This field is used for
-// supporting docker private registry credential per container.
-// Users can specify different docker config files for pulling
-// their private images from different registries.
+// Docker config containing credentails to authenticate with
+// docker registry. The secret is expected to be a docker
+// config file in JSON format with UTF-8 character encoding.
 optional Secret config = 3;
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/90385489/include/mesos/v1/mesos.proto
--
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index dc58acc..bc7dbe0 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -2144,16 +2144,14 @@ message Image {
 // NOTE: This is not encrypted, therefore framework and operators
 // should enable SSL when passing this information.
 //
-// This is field is never used in Mesos before and is deprecated
-// since Mesos 1.3. Please use the `Secret::Value` field `config`
-// below (see MESOS-7088 for details).
+// This field has never been used in Mesos before and is
+// deprecated since Mesos 1.3. Please use `config` below
+// (see MESOS-7088 for details).
 optional Credential credential = 2 [deprecated = true]; // Since 1.3.
 
-// The UTF-8 character encoded byte data, which is expected as
-// a docker config file in JSON format. This field is used for
-// supporting docker private registry credential per container.
-// Users can specify different docker config files for pulling
-// their private images from different registries.
+// Docker config containing credentails to authenticate with
+// docker registry. The secret is expected to be a docker
+// config file in JSON format with UTF-8 character encoding.
 optional Secret config = 3;
   }
 

[05/10] mesos git commit: Fixed docker/appc store 'using' format.

[gilbert]

Fixed docker/appc store 'using' format.

Review: https://reviews.apache.org/r/59011


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/32dc11ab
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/32dc11ab
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/32dc11ab

Branch: refs/heads/master
Commit: 32dc11abd74c96a02c52e71448823d93207817f9
Parents: 9038548
Author: Gilbert Song 
Authored: Mon May 1 16:42:46 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 include/mesos/appc/spec.hpp   |  1 +
 .../containerizer/mesos/provisioner/appc/store.cpp| 12 ++--
 .../containerizer/mesos/provisioner/docker/store.cpp  | 14 --
 src/slave/containerizer/mesos/provisioner/store.cpp   |  4 ++--
 4 files changed, 25 insertions(+), 6 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/32dc11ab/include/mesos/appc/spec.hpp
--
diff --git a/include/mesos/appc/spec.hpp b/include/mesos/appc/spec.hpp
index e9430df..0fef217 100644
--- a/include/mesos/appc/spec.hpp
+++ b/include/mesos/appc/spec.hpp
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/32dc11ab/src/slave/containerizer/mesos/provisioner/appc/store.cpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/appc/store.cpp 
b/src/slave/containerizer/mesos/provisioner/appc/store.cpp
index 09a40a5..dc547dd 100644
--- a/src/slave/containerizer/mesos/provisioner/appc/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/appc/store.cpp
@@ -35,15 +35,23 @@
 #include "slave/containerizer/mesos/provisioner/appc/paths.hpp"
 #include "slave/containerizer/mesos/provisioner/appc/store.hpp"
 
-using namespace process;
-
 namespace spec = appc::spec;
 
 using std::list;
 using std::string;
 using std::vector;
 
+using process::Failure;
+using process::Future;
 using process::Owned;
+using process::Process;
+using process::Promise;
+
+using process::defer;
+using process::dispatch;
+using process::spawn;
+using process::terminate;
+using process::wait;
 
 namespace mesos {
 namespace internal {

http://git-wip-us.apache.org/repos/asf/mesos/blob/32dc11ab/src/slave/containerizer/mesos/provisioner/docker/store.cpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/docker/store.cpp 
b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
index 68ce265..7529afd 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -40,14 +40,24 @@
 
 #include "uri/fetcher.hpp"
 
-using namespace process;
-
 namespace spec = docker::spec;
 
 using std::list;
 using std::string;
 using std::vector;
 
+using process::Failure;
+using process::Future;
+using process::Owned;
+using process::Process;
+using process::Promise;
+
+using process::defer;
+using process::dispatch;
+using process::spawn;
+using process::terminate;
+using process::wait;
+
 namespace mesos {
 namespace internal {
 namespace slave {

http://git-wip-us.apache.org/repos/asf/mesos/blob/32dc11ab/src/slave/containerizer/mesos/provisioner/store.cpp
--
diff --git a/src/slave/containerizer/mesos/provisioner/store.cpp 
b/src/slave/containerizer/mesos/provisioner/store.cpp
index 7141d63..260a746 100644
--- a/src/slave/containerizer/mesos/provisioner/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/store.cpp
@@ -28,10 +28,10 @@
 
 #include "slave/containerizer/mesos/provisioner/docker/store.hpp"
 
-using namespace process;
-
 using std::string;
 
+using process::Owned;
+
 namespace mesos {
 namespace internal {
 namespace slave {

[10/10] mesos git commit: Added new parameter 'data' to the URI fetcher interface.

[gilbert]

Added new parameter 'data' to the URI fetcher interface.

Review: https://reviews.apache.org/r/59141


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/92f16fc2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/92f16fc2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/92f16fc2

Branch: refs/heads/master
Commit: 92f16fc22b78abc2f407d1730e901e6890d31bf5
Parents: 618a101
Author: Gilbert Song 
Authored: Wed May 10 04:45:53 2017 -0700
Committer: Gilbert Song 
Committed: Thu May 25 01:04:30 2017 +0800

--
 include/mesos/uri/fetcher.hpp   | 16 +---
 .../mesos/provisioner/docker/registry_puller.cpp| 10 --
 src/tests/uri_fetcher_tests.cpp | 10 ++
 src/uri/fetcher.cpp | 10 ++
 src/uri/fetchers/copy.cpp   |  3 ++-
 src/uri/fetchers/copy.hpp   |  3 ++-
 src/uri/fetchers/curl.cpp   |  3 ++-
 src/uri/fetchers/curl.hpp   |  3 ++-
 src/uri/fetchers/docker.cpp |  8 ++--
 src/uri/fetchers/docker.hpp |  3 ++-
 src/uri/fetchers/hadoop.cpp |  3 ++-
 src/uri/fetchers/hadoop.hpp |  3 ++-
 12 files changed, 53 insertions(+), 22 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/92f16fc2/include/mesos/uri/fetcher.hpp
--
diff --git a/include/mesos/uri/fetcher.hpp b/include/mesos/uri/fetcher.hpp
index ebf86c7..760d6b3 100644
--- a/include/mesos/uri/fetcher.hpp
+++ b/include/mesos/uri/fetcher.hpp
@@ -21,6 +21,8 @@
 #include 
 #include 
 
+#include 
+
 #include 
 #include 
 #include 
@@ -69,10 +71,14 @@ public:
  *
  * @param uri the URI to fetch
  * @param directory the directory the URI will be downloaded to
+ * @param data the optional user defined data
  */
+// TODO(gilbert): Change the parameter 'data' as a hashmap
+// of , and update the comment.
 virtual process::Future fetch(
 const URI& uri,
-const std::string& directory) const = 0;
+const std::string& directory,
+const Option& data = None()) const = 0;
   };
 
   /**
@@ -88,11 +94,13 @@ public:
*
* @param uri the URI to fetch
* @param directory the directory the URI will be downloaded to
+   * @param data the optional user defined data
*/
   // TODO(jieyu): Consider using 'Path' for 'directory' here.
   process::Future fetch(
   const URI& uri,
-  const std::string& directory) const;
+  const std::string& directory,
+  const Option& data = None()) const;
 
   /**
* Fetches a URI to the given directory. This method will dispatch
@@ -101,11 +109,13 @@ public:
* @param uri the URI to fetch
* @param directory the directory the URI will be downloaded to
* @param name of the plugin that is used to download
+   * @param data the optional user defined data
*/
   process::Future fetch(
   const URI& uri,
   const std::string& directory,
-  const std::string& name) const;
+  const std::string& name,
+  const Option& data = None()) const;
 
 private:
   Fetcher(const Fetcher&) = delete; // Not copyable.

http://git-wip-us.apache.org/repos/asf/mesos/blob/92f16fc2/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
--
diff --git 
a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index a5f299a..693e841 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -288,7 +288,10 @@ Future RegistryPullerProcess::_pull(
   << "' from '" << manifestUri
   << "' to '" << directory << "'";
 
-  return fetcher->fetch(manifestUri, directory)
+  return fetcher->fetch(
+  manifestUri,
+  directory,
+  config.isSome() ? config->data() : Option())
 .then(defer(self(), ::__pull, reference, directory, backend, config));
 }
 
@@ -504,7 +507,10 @@ Future RegistryPullerProcess::fetchBlobs(
   port);
 }
 
-futures.push_back(fetcher->fetch(blobUri, directory));
+futures.push_back(fetcher->fetch(
+blobUri,
+directory,
+config.isSome() ? config->data() : Option()));
   }
 
   return collect(futures)

http://git-wip-us.apache.org/repos/asf/mesos/blob/92f16fc2/src/tests/uri_fetcher_tests.cpp

mesos git commit: Renamed RegisterAgent.agent to RegisterAgent.agents in acls.proto.

[neilc]

Repository: mesos
Updated Branches:
  refs/heads/1.3.x e9e759aac -> 230a08c73


Renamed RegisterAgent.agent to RegisterAgent.agents in acls.proto.

Renames the field `RegisterAgent.agent` to `RegisterAgent.agents` in
order to come make it consistent with other ACLs.

Review: https://reviews.apache.org/r/59453/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/230a08c7
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/230a08c7
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/230a08c7

Branch: refs/heads/1.3.x
Commit: 230a08c73676cac7a5c239e99a137125b0d7ef79
Parents: e9e759a
Author: Alexander Rojas 
Authored: Wed May 24 09:24:20 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 09:43:33 2017 -0700

--
 include/mesos/authorizer/acls.proto  | 2 +-
 src/authorizer/local/authorizer.cpp  | 6 +++---
 src/tests/authorization_tests.cpp| 6 +++---
 src/tests/master_authorization_tests.cpp | 8 
 src/tests/script.cpp | 2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/230a08c7/include/mesos/authorizer/acls.proto
--
diff --git a/include/mesos/authorizer/acls.proto 
b/include/mesos/authorizer/acls.proto
index ae0b1ea..36b3ac2 100644
--- a/include/mesos/authorizer/acls.proto
+++ b/include/mesos/authorizer/acls.proto
@@ -351,7 +351,7 @@ message ACL {
 
 // Objects: Given implicitly. Use Entity type ANY or NONE to allow or deny
 // access.
-required Entity agent = 2;
+required Entity agents = 2;
   }
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/230a08c7/src/authorizer/local/authorizer.cpp
--
diff --git a/src/authorizer/local/authorizer.cpp 
b/src/authorizer/local/authorizer.cpp
index 89aaf4b..1f2a990 100644
--- a/src/authorizer/local/authorizer.cpp
+++ b/src/authorizer/local/authorizer.cpp
@@ -1242,7 +1242,7 @@ private:
 foreach (const ACL::RegisterAgent& acl, acls.register_agents()) {
   GenericACL acl_;
   acl_.subjects = acl.principals();
-  acl_.objects = acl.agent();
+  acl_.objects = acl.agents();
 
   acls_.push_back(acl_);
 }
@@ -1337,9 +1337,9 @@ Option LocalAuthorizer::validate(const ACLs& acls)
   }
 
   foreach (const ACL::RegisterAgent& acl, acls.register_agents()) {
-if (acl.agent().type() == ACL::Entity::SOME) {
+if (acl.agents().type() == ACL::Entity::SOME) {
   return Error(
-  "acls.register_agents.agent type must be either NONE or ANY");
+  "acls.register_agents type must be either NONE or ANY");
 }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/230a08c7/src/tests/authorization_tests.cpp
--
diff --git a/src/tests/authorization_tests.cpp 
b/src/tests/authorization_tests.cpp
index 32aa6ac..6d85a54 100644
--- a/src/tests/authorization_tests.cpp
+++ b/src/tests/authorization_tests.cpp
@@ -4898,14 +4898,14 @@ TYPED_TEST(AuthorizationTest, RegisterAgent)
 // "foo" principal can register as an agent.
 mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
 acl->mutable_principals()->add_values("foo");
-acl->mutable_agent()->set_type(mesos::ACL::Entity::ANY);
+acl->mutable_agents()->set_type(mesos::ACL::Entity::ANY);
   }
 
   {
 // Nobody else can register as an agent.
 mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
 acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
-acl->mutable_agent()->set_type(mesos::ACL::Entity::NONE);
+acl->mutable_agents()->set_type(mesos::ACL::Entity::NONE);
   }
 
   Try create = TypeParam::create(parameterize(acls));
@@ -4936,7 +4936,7 @@ TYPED_TEST(AuthorizationTest, RegisterAgent)
 
 mesos::ACL::RegisterAgent* acl = invalid.add_register_agents();
 acl->mutable_principals()->add_values("foo");
-acl->mutable_agent()->add_values("yoda");
+acl->mutable_agents()->add_values("yoda");
 
 Try create = TypeParam::create(parameterize(invalid));
 EXPECT_ERROR(create);

http://git-wip-us.apache.org/repos/asf/mesos/blob/230a08c7/src/tests/master_authorization_tests.cpp
--
diff --git a/src/tests/master_authorization_tests.cpp 
b/src/tests/master_authorization_tests.cpp
index e4233c1..0a2f31b 100644
--- a/src/tests/master_authorization_tests.cpp
+++ b/src/tests/master_authorization_tests.cpp
@@ -2339,7 +2339,7 @@ TEST_F(MasterAuthorizationTest, 
AuthorizedToRegisterAndReregisterAgent)
   ACLs acls;
   

[2/8] mesos git commit: Added a new sorter test case, `HierarchicalIterationOrder`.

[neilc]

Added a new sorter test case, `HierarchicalIterationOrder`.

This behavior is covered by existing test cases to an extent, but some
additional test coverage seems warranted.

Review: https://reviews.apache.org/r/59482/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c19ad9a0
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c19ad9a0
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c19ad9a0

Branch: refs/heads/1.3.x
Commit: c19ad9a0ad99d5fb6f6cb75a4f2161481491007d
Parents: 7a8cd22
Author: Neil Conway 
Authored: Tue May 23 10:36:13 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:46:55 2017 -0700

--
 src/tests/sorter_tests.cpp | 48 +
 1 file changed, 48 insertions(+)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/c19ad9a0/src/tests/sorter_tests.cpp
--
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index d8d6140..4891806 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -605,6 +605,54 @@ TEST(SorterTest, HierarchicalAllocation)
 }
 
 
+// This test checks that the sorted list of clients returned by the
+// sorter iterates over the client tree in the correct order.
+TEST(SorterTest, HierarchicalIterationOrder)
+{
+  DRFSorter sorter;
+
+  SlaveID slaveId;
+  slaveId.set_value("agentId");
+
+  Resources totalResources = Resources::parse("cpus:100;mem:100").get();
+  sorter.add(slaveId, totalResources);
+
+  sorter.add("a/b");
+  sorter.add("c");
+  sorter.add("d");
+  sorter.add("d/e");
+
+  sorter.activate("a/b");
+  sorter.activate("c");
+  sorter.activate("d");
+  sorter.activate("d/e");
+
+  // Shares: a/b = 0, c = 0, d = 0, d/e = 0
+  EXPECT_EQ(vector({"a/b", "c", "d", "d/e"}), sorter.sort());
+
+  Resources cResources = Resources::parse("cpus:8;mem:8").get();
+  sorter.allocated("c", slaveId, cResources);
+
+  // Shares: a/b = 0, d = 0, d/e = 0, c = 0.08.
+  EXPECT_EQ(vector({"a/b", "d", "d/e", "c"}), sorter.sort());
+
+  Resources dResources = Resources::parse("cpus:3;mem:3").get();
+  sorter.allocated("d", slaveId, dResources);
+
+  Resources deResources = Resources::parse("cpus:2;mem:2").get();
+  sorter.allocated("d/e", slaveId, deResources);
+
+  // Shares: a/b = 0, d/e = 0.02, d = 0.03, c = 0.08.
+  EXPECT_EQ(vector({"a/b", "d/e", "d", "c"}), sorter.sort());
+
+  Resources abResources = Resources::parse("cpus:6;mem:6").get();
+  sorter.allocated("a/b", slaveId, abResources);
+
+  // Shares: d/e = 0.02, d = 0.03, a/b = 0.06, c = 0.08.
+  EXPECT_EQ(vector({"d/e", "d", "a/b", "c"}), sorter.sort());
+}
+
+
 // This test checks what happens when a new sorter client is added as
 // a child of what was previously a leaf node.
 TEST(SorterTest, AddChildToLeaf)

[4/8] mesos git commit: Introduced `DRFSorter::Node::isLeaf()`.

[neilc]

Introduced `DRFSorter::Node::isLeaf()`.

This helps clarify code that wants to distinguish between leaves and
internal nodes in the sorter.

Review: https://reviews.apache.org/r/59484/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b48464f0
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b48464f0
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b48464f0

Branch: refs/heads/1.3.x
Commit: b48464f0381a7280bab2b5b99e6a361666b2a593
Parents: 68019b5
Author: Neil Conway 
Authored: Tue May 23 10:36:17 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 15:19:15 2017 -0700

--
 src/master/allocator/sorter/drf/sorter.cpp | 22 ++
 src/master/allocator/sorter/drf/sorter.hpp | 10 ++
 2 files changed, 16 insertions(+), 16 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/b48464f0/src/master/allocator/sorter/drf/sorter.cpp
--
diff --git a/src/master/allocator/sorter/drf/sorter.cpp 
b/src/master/allocator/sorter/drf/sorter.cpp
index 74da5b1..1d8959d 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -99,13 +99,7 @@ void DRFSorter::add(const string& clientPath)
 // leaf node into an internal node, we need to create an
 // additional child node: `current` must have been associated with
 // a client and clients must always be associated with leaf nodes.
-//
-// There are two exceptions: if `current` is the root node or it
-// was just created by the current `add()` call, it does not
-// correspond to a client, so we don't create an extra child.
-if (current->children.empty() &&
-current != root &&
-current != lastCreatedNode) {
+if (current->isLeaf()) {
   Node* parent = CHECK_NOTNULL(current->parent);
 
   parent->removeChild(current);
@@ -122,9 +116,10 @@ void DRFSorter::add(const string& clientPath)
   // `internal`.
   current->name = ".";
   current->parent = internal;
-  internal->addChild(current);
   current->path = strings::join("/", parent->path, current->name);
 
+  internal->addChild(current);
+
   CHECK_EQ(internal->path, current->clientPath());
 
   current = internal;
@@ -155,8 +150,7 @@ void DRFSorter::add(const string& clientPath)
   }
 
   // `current` is the newly created node associated with the last
-  // element of the path. `current` should be an inactive node with no
-  // children.
+  // element of the path. `current` should be an inactive leaf node.
   CHECK(current->children.empty());
   CHECK(current->kind == Node::INACTIVE_LEAF);
 
@@ -228,9 +222,7 @@ void DRFSorter::remove(const string& clientPath)
   Node* child = *(current->children.begin());
 
   if (child->name == ".") {
-CHECK(child->children.empty());
-CHECK(child->kind == Node::ACTIVE_LEAF ||
-  child->kind == Node::INACTIVE_LEAF);
+CHECK(child->isLeaf());
 CHECK(clients.contains(current->path));
 CHECK_EQ(child, clients.at(current->path));
 
@@ -578,9 +570,7 @@ DRFSorter::Node* DRFSorter::find(const string& clientPath) 
const
 
   Node* client = client_.get();
 
-  CHECK(client->kind == Node::ACTIVE_LEAF ||
-client->kind == Node::INACTIVE_LEAF);
-  CHECK(client->children.empty());
+  CHECK(client->isLeaf());
 
   return client;
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/b48464f0/src/master/allocator/sorter/drf/sorter.hpp
--
diff --git a/src/master/allocator/sorter/drf/sorter.hpp 
b/src/master/allocator/sorter/drf/sorter.hpp
index f76d2f7..67700d5 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -267,6 +267,16 @@ struct DRFSorter::Node
 return path;
   }
 
+  bool isLeaf() const
+  {
+if (kind == ACTIVE_LEAF || kind == INACTIVE_LEAF) {
+  CHECK(children.empty());
+  return true;
+}
+
+return false;
+  }
+
   void removeChild(const Node* child)
   {
 auto it = std::find(children.begin(), children.end(), child);

[8/8] mesos git commit: Added MESOS-7521 to 1.3.0 CHANGELOG.

[neilc]

Added MESOS-7521 to 1.3.0 CHANGELOG.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/904729ed
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/904729ed
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/904729ed

Branch: refs/heads/1.3.x
Commit: 904729ed37802545ec650e46177e49e1c35c84a8
Parents: 3c64c0d
Author: Neil Conway 
Authored: Wed May 24 15:20:28 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 15:20:28 2017 -0700

--
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/904729ed/CHANGELOG
--
diff --git a/CHANGELOG b/CHANGELOG
index 5a41e53..0d8c480 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -199,6 +199,7 @@ All Resolved Issues:
   * [MESOS-7471] - Provisioner recover should not always assume 'rootfses' dir 
exists.
   * [MESOS-7478] - Pre-1.2.x master does not work with 1.2.x agent.
   * [MESOS-7484] - VersionTest.ParseInvalid aborts on Windows.
+  * [MESOS-7521] - Major performance regression in DRF sorter.
   * [MESOS-7538] - Don't validate re-registrations that are going to be 
dropped.
 
 ** Documentation

[3/8] mesos git commit: Added benchmark for allocator perf with many suppressed frameworks.

[neilc]

Added benchmark for allocator perf with many suppressed frameworks.

This covers the case where the vast majority (99%) of frameworks have
suppressed offers.

Review: https://reviews.apache.org/r/59383/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/89aaa05c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/89aaa05c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/89aaa05c

Branch: refs/heads/1.3.x
Commit: 89aaa05cd6e6f672f0c3750821bcc77446564797
Parents: c19ad9a
Author: Neil Conway 
Authored: Tue May 23 10:36:14 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 15:19:14 2017 -0700

--
 src/tests/hierarchical_allocator_tests.cpp | 150 
 1 file changed, 150 insertions(+)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/89aaa05c/src/tests/hierarchical_allocator_tests.cpp
--
diff --git a/src/tests/hierarchical_allocator_tests.cpp 
b/src/tests/hierarchical_allocator_tests.cpp
index ebc4868..2f23080 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -5416,6 +5416,156 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
SuppressOffers)
 }
 
 
+// This benchmark measures allocator performance when almost all
+// frameworks are suppressed.
+TEST_P(HierarchicalAllocator_BENCHMARK_Test, ExtremeSuppressOffers)
+{
+  size_t agentCount = std::tr1::get<0>(GetParam());
+  size_t frameworkCount = std::tr1::get<1>(GetParam());
+
+  // Pause the clock because we want to manually drive the allocations.
+  Clock::pause();
+
+  struct OfferedResources
+  {
+FrameworkID   frameworkId;
+SlaveID   slaveId;
+Resources resources;
+  };
+
+  vector offers;
+
+  auto offerCallback = [](
+  const FrameworkID& frameworkId,
+  const hashmap>& resources_)
+  {
+foreachkey (const string& role, resources_) {
+  foreachpair (const SlaveID& slaveId,
+   const Resources& resources,
+   resources_.at(role)) {
+offers.push_back(OfferedResources{frameworkId, slaveId, resources});
+  }
+}
+  };
+
+  cout << "Using " << agentCount << " agents and "
+   << frameworkCount << " frameworks" << endl;
+
+  master::Flags flags;
+  initialize(flags, offerCallback);
+
+  vector frameworks;
+  frameworks.reserve(frameworkCount);
+
+  Stopwatch watch;
+  watch.start();
+
+  for (size_t i = 0; i < frameworkCount; i++) {
+frameworks.push_back(createFrameworkInfo({"*"}));
+allocator->addFramework(frameworks[i].id(), frameworks[i], {}, true);
+  }
+
+  // Wait for all the `addFramework` operations to be processed.
+  Clock::settle();
+
+  watch.stop();
+
+  cout << "Added " << frameworkCount << " frameworks"
+   << " in " << watch.elapsed() << endl;
+
+  vector agents;
+  agents.reserve(agentCount);
+
+  const Resources agentResources = Resources::parse(
+  "cpus:24;mem:4096;disk:4096;ports:[31000-32000]").get();
+
+  // Each agent has a portion of its resources allocated to a single
+  // framework. We round-robin through the frameworks when allocating.
+  Resources allocation = Resources::parse("cpus:16;mem:1024;disk:1024").get();
+
+  Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 32000), 16);
+  ASSERT_SOME(ranges);
+  ASSERT_EQ(16, ranges->range_size());
+
+  allocation += createPorts(ranges.get());
+  allocation.allocate("*");
+
+  watch.start();
+
+  for (size_t i = 0; i < agentCount; i++) {
+agents.push_back(createSlaveInfo(agentResources));
+
+hashmap used;
+used[frameworks[i % frameworkCount].id()] = allocation;
+
+allocator->addSlave(
+agents[i].id(),
+agents[i],
+AGENT_CAPABILITIES(),
+None(),
+agents[i].resources(),
+used);
+  }
+
+  // Wait for all the `addSlave` operations to be processed.
+  Clock::settle();
+
+  watch.stop();
+
+  cout << "Added " << agentCount << " agents"
+   << " in " << watch.elapsed() << endl;
+
+  // Now perform allocations. Each time we trigger an allocation run, we
+  // increase the number of frameworks that are suppressing offers. To
+  // ensure the test can run in a timely manner, we always perform a
+  // fixed number of allocations.
+  //
+  // TODO(jjanco): Parameterize this test by allocationsCount, not an arbitrary
+  // number. Batching reduces loop size, lowering time to test completion.
+  size_t allocationsCount = 5;
+
+  // Suppress offers for 99% of frameworks.
+  size_t suppressCount = static_cast(frameworkCount * 0.99);
+  CHECK(suppressCount < frameworkCount);
+
+  for (size_t i = 0; i < suppressCount; i++) {
+

[1/8] mesos git commit: Added sorter test for allocation queries about inactive clients.

[neilc]

Repository: mesos
Updated Branches:
  refs/heads/1.3.x 230a08c73 -> 904729ed3


Added sorter test for allocation queries about inactive clients.

This behavior has not changed, but was not covered by the existing
tests.

Review: https://reviews.apache.org/r/59481/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7a8cd22b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7a8cd22b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7a8cd22b

Branch: refs/heads/1.3.x
Commit: 7a8cd22b1df566a0213483a77aeb626266618ff2
Parents: 230a08c
Author: Neil Conway 
Authored: Tue May 23 10:36:12 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:46:49 2017 -0700

--
 src/tests/sorter_tests.cpp | 37 +
 1 file changed, 37 insertions(+)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/7a8cd22b/src/tests/sorter_tests.cpp
--
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 8e7ff79..d8d6140 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -989,6 +989,43 @@ TEST(SorterTest, UpdateAllocationNestedClient)
 }
 
 
+// This test checks that the sorter correctly reports allocation
+// information about inactive clients.
+TEST(SorterTest, AllocationForInactiveClient)
+{
+  DRFSorter sorter;
+
+  SlaveID slaveId;
+  slaveId.set_value("agentId");
+
+  sorter.add(slaveId, Resources::parse("cpus:10;mem:10").get());
+
+  sorter.add("a");
+  sorter.add("b");
+
+  // Leave client "a" inactive.
+  sorter.activate("b");
+
+  sorter.allocated("a", slaveId, Resources::parse("cpus:2;mem:2").get());
+  sorter.allocated("b", slaveId, Resources::parse("cpus:3;mem:3").get());
+
+  hashmap clientAllocation = sorter.allocation(slaveId);
+  EXPECT_EQ(2u, clientAllocation.size());
+  EXPECT_EQ(Resources::parse("cpus:2;mem:2").get(), clientAllocation.at("a"));
+  EXPECT_EQ(Resources::parse("cpus:3;mem:3").get(), clientAllocation.at("b"));
+
+  hashmap agentAllocation1 = sorter.allocation("a");
+  EXPECT_EQ(1u, agentAllocation1.size());
+  EXPECT_EQ(
+  Resources::parse("cpus:2;mem:2").get(), agentAllocation1.at(slaveId));
+
+  hashmap agentAllocation2 = sorter.allocation("b");
+  EXPECT_EQ(1u, agentAllocation2.size());
+  EXPECT_EQ(
+  Resources::parse("cpus:3;mem:3").get(), agentAllocation2.at(slaveId));
+}
+
+
 // We aggregate resources from multiple slaves into the sorter.
 // Since non-scalar resources don't aggregate well across slaves,
 // we need to keep track of the SlaveIDs of the resources. This

[5/8] mesos git commit: Optimized sorter performance with many inactive clients.

[neilc]

Optimized sorter performance with many inactive clients.

Unlike in Mesos <= 1.2, the sorter now stores inactive clients in the
same data structure used to store active clients. This resulted in a
significant performance regression when the vast majority of sorter
clients are inactive: when sorting clients and producing the sorted
order, we iterate over ALL clients (active and inactive), which could be
much slower than the old active-only implementation.

This commit revises the sorter to ensure that inactive leaf nodes are
always stored at the end of their parent's list of child nodes. This
allows the sorter to stop early (at the first inactive leaf) when
iterating over a node's children, if we're only interested in applying
an operation to each active leaf or internal node. This change fixes the
observed performance regression relative to Mesos 1.2.0.

Review: https://reviews.apache.org/r/59355/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3c64c0d4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3c64c0d4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3c64c0d4

Branch: refs/heads/1.3.x
Commit: 3c64c0d42fe5c0c072c69ab1fe155a21419c0e98
Parents: b48464f
Author: Neil Conway 
Authored: Tue May 23 10:36:18 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 15:19:15 2017 -0700

--
 src/master/allocator/sorter/drf/sorter.cpp | 98 +
 src/master/allocator/sorter/drf/sorter.hpp | 27 ++-
 2 files changed, 110 insertions(+), 15 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/3c64c0d4/src/master/allocator/sorter/drf/sorter.cpp
--
diff --git a/src/master/allocator/sorter/drf/sorter.cpp 
b/src/master/allocator/sorter/drf/sorter.cpp
index 1d8959d..ecc5586 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -147,6 +147,14 @@ void DRFSorter::add(const string& clientPath)
 // If we created `current` in the loop above, it was marked an
 // `INTERNAL` node. It should actually be an inactive leaf node.
 current->kind = Node::INACTIVE_LEAF;
+
+// `current` has changed from an internal node to an inactive
+// leaf, so remove and re-add it to its parent. This moves it to
+// the end of the parent's list of children.
+CHECK_NOTNULL(current->parent);
+
+current->parent->removeChild(current);
+current->parent->addChild(current);
   }
 
   // `current` is the newly created node associated with the last
@@ -229,6 +237,16 @@ void DRFSorter::remove(const string& clientPath)
 current->kind = child->kind;
 current->removeChild(child);
 
+// `current` has changed kind (from `INTERNAL` to a leaf,
+// which might be active or inactive). Hence we might need to
+// change its position in the `children` list.
+if (current->kind == Node::INTERNAL) {
+  CHECK_NOTNULL(current->parent);
+
+  current->parent->removeChild(current);
+  current->parent->addChild(current);
+}
+
 clients[current->path] = current;
 
 delete child;
@@ -250,14 +268,41 @@ void DRFSorter::remove(const string& clientPath)
 void DRFSorter::activate(const string& clientPath)
 {
   Node* client = CHECK_NOTNULL(find(clientPath));
-  client->kind = Node::ACTIVE_LEAF;
+
+  if (client->kind == Node::INACTIVE_LEAF) {
+client->kind = Node::ACTIVE_LEAF;
+
+// `client` has been activated, so move it to the beginning of its
+// parent's list of children. We mark the tree dirty, so that the
+// client's share is updated correctly and it is sorted properly.
+//
+// TODO(neilc): We could instead calculate share here and insert
+// the client into the appropriate place here, which would avoid
+// dirtying the whole tree.
+CHECK_NOTNULL(client->parent);
+
+client->parent->removeChild(client);
+client->parent->addChild(client);
+
+dirty = true;
+  }
 }
 
 
 void DRFSorter::deactivate(const string& clientPath)
 {
   Node* client = CHECK_NOTNULL(find(clientPath));
-  client->kind = Node::INACTIVE_LEAF;
+
+  if (client->kind == Node::ACTIVE_LEAF) {
+client->kind = Node::INACTIVE_LEAF;
+
+// `client` has been deactivated, so move it to the end of its
+// parent's list of children.
+CHECK_NOTNULL(client->parent);
+
+client->parent->removeChild(client);
+client->parent->addChild(client);
+  }
 }
 
 
@@ -467,16 +512,33 @@ vector DRFSorter::sort()
 {
   if (dirty) {
 std::function sortTree = [this, ](Node* node) {
-  foreach (Node* child, node->children) {
+  // Inactive leaves are always stored at the end of the
+  // `children` vector; this 

[6/8] mesos git commit: Cleaned up allocator benchmark tests.

[neilc]

Cleaned up allocator benchmark tests.

Review: https://reviews.apache.org/r/59538


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/07a339f4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/07a339f4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/07a339f4

Branch: refs/heads/1.3.x
Commit: 07a339f41a929a0f04393b9ece1573fc61aaa2b6
Parents: 89aaa05
Author: Neil Conway 
Authored: Wed May 24 13:19:55 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 15:19:15 2017 -0700

--
 src/tests/hierarchical_allocator_tests.cpp | 36 ++---
 1 file changed, 21 insertions(+), 15 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/07a339f4/src/tests/hierarchical_allocator_tests.cpp
--
diff --git a/src/tests/hierarchical_allocator_tests.cpp 
b/src/tests/hierarchical_allocator_tests.cpp
index 2f23080..68b01cd 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -4886,9 +4886,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
AddAndUpdateSlave)
   // Add the slaves, use round-robin to choose which framework
   // to allocate a slice of the slave's resources to.
   for (size_t i = 0; i < slaves.size(); i++) {
-hashmap used;
-
-used[frameworks[i % frameworkCount].id()] = allocation;
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), allocation}
+};
 
 allocator->addSlave(
 slaves[i].id(),
@@ -5011,10 +5011,12 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
DeclineOffers)
   for (size_t i = 0; i < slaveCount; i++) {
 slaves.push_back(createSlaveInfo(agentResources));
 
-// Add some used resources on each slave. Let's say there are 16 tasks, 
each
-// is allocated 1 cpu and a random port from the port range.
-hashmap used;
-used[frameworks[i % frameworkCount].id()] = allocation;
+// Add some used resources on each slave. Let's say there are 16 tasks;
+// each is allocated 1 cpu and a random port from the port range.
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), allocation}
+};
+
 allocator->addSlave(
 slaves[i].id(),
 slaves[i],
@@ -5204,8 +5206,10 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
ResourceLabels)
 
 // Add some used resources on each slave. Let's say there are 16 tasks, 
each
 // is allocated 1 cpu and a random port from the port range.
-hashmap used;
-used[frameworks[i % frameworkCount].id()] = _allocation;
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), _allocation}
+};
+
 allocator->addSlave(
 slaves[i].id(),
 slaves[i],
@@ -5339,8 +5343,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
SuppressOffers)
   for (size_t i = 0; i < agentCount; i++) {
 agents.push_back(createSlaveInfo(agentResources));
 
-hashmap used;
-used[frameworks[i % frameworkCount].id()] = allocation;
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), allocation}
+};
 
 allocator->addSlave(
 agents[i].id(),
@@ -5369,7 +5374,7 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
SuppressOffers)
   size_t allocationsCount = 5;
   size_t suppressCount = 0;
 
-  for (size_t i = 0; i < allocationsCount; ++i) {
+  for (size_t i = 0; i < allocationsCount; i++) {
 // Recover resources with no filters because we want to test the
 // effect of suppression alone.
 foreach (const OfferedResources& offer, offers) {
@@ -5495,8 +5500,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
ExtremeSuppressOffers)
   for (size_t i = 0; i < agentCount; i++) {
 agents.push_back(createSlaveInfo(agentResources));
 
-hashmap used;
-used[frameworks[i % frameworkCount].id()] = allocation;
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), allocation}
+};
 
 allocator->addSlave(
 agents[i].id(),
@@ -5532,7 +5538,7 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
ExtremeSuppressOffers)
 allocator->suppressOffers(frameworks[i].id(), {});
   }
 
-  for (size_t i = 0; i < allocationsCount; ++i) {
+  for (size_t i = 0; i < allocationsCount; i++) {
 // Recover resources with no filters because we want to test the
 // effect of suppression alone.
 foreach (const OfferedResources& offer, offers) {

[7/8] mesos git commit: Replaced the sorter's notion of "activation" with a three-valued enum.

[neilc]

Replaced the sorter's notion of "activation" with a three-valued enum.

Conceptually, a node in the sorter tree can either be an internal node
or a leaf node. Furthermore, leaf nodes can either be active or inactive
(internal nodes do not have a concept of "activation").

We previously represented this situation with a single boolean,
`active`. The boolean was true for active leaves, and false for inactive
leaves and internal nodes. Whether a node was an internal node was
determined by checking the number of children it has.

This commit replaces `active` with a three-valued enum named `kind`,
which can take on the values `ACTIVE_LEAF`, `INACTIVE_LEAF`, or
`INTERNAL`. This enforces the idea that internal nodes do not have a
notion of "activation", and more explicitly distinguishes between leaf
and internal nodes.

Review: https://reviews.apache.org/r/59483/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/68019b5b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/68019b5b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/68019b5b

Branch: refs/heads/1.3.x
Commit: 68019b5ba6f5cca8cd17ee2c4ad258633e3d7f69
Parents: 07a339f
Author: Neil Conway 
Authored: Tue May 23 10:36:16 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 15:19:15 2017 -0700

--
 src/master/allocator/sorter/drf/sorter.cpp | 40 +
 src/master/allocator/sorter/drf/sorter.hpp | 22 +-
 2 files changed, 42 insertions(+), 20 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/68019b5b/src/master/allocator/sorter/drf/sorter.cpp
--
diff --git a/src/master/allocator/sorter/drf/sorter.cpp 
b/src/master/allocator/sorter/drf/sorter.cpp
index 26b77f5..74da5b1 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -45,13 +45,13 @@ namespace allocator {
 
 
 DRFSorter::DRFSorter()
-  : root(new Node("", nullptr)) {}
+  : root(new Node("", Node::INTERNAL, nullptr)) {}
 
 
 DRFSorter::DRFSorter(
 const UPID& allocator,
 const string& metricsPrefix)
-  : root(new Node("", nullptr)),
+  : root(new Node("", Node::INTERNAL, nullptr)),
 metrics(Metrics(allocator, *this, metricsPrefix)) {}
 
 
@@ -112,7 +112,7 @@ void DRFSorter::add(const string& clientPath)
 
   // Create a node under `parent`. This internal node will take
   // the place of `current` in the tree.
-  Node* internal = new Node(current->name, parent);
+  Node* internal = new Node(current->name, Node::INTERNAL, parent);
   parent->addChild(internal);
   internal->allocation = current->allocation;
 
@@ -131,28 +131,34 @@ void DRFSorter::add(const string& clientPath)
 }
 
 // Now actually add a new child to `current`.
-Node* newChild = new Node(element, current);
+Node* newChild = new Node(element, Node::INTERNAL, current);
 current->addChild(newChild);
 
 current = newChild;
 lastCreatedNode = newChild;
   }
 
+  CHECK(current->kind == Node::INTERNAL);
+
   // `current` is the node associated with the last element of the
   // path. If we didn't add `current` to the tree above, create a leaf
   // node now. For example, if the tree contains "a/b" and we add a
   // new client "a", we want to create a new leaf node "a/." here.
   if (current != lastCreatedNode) {
-Node* newChild = new Node(".", current);
+Node* newChild = new Node(".", Node::INACTIVE_LEAF, current);
 current->addChild(newChild);
 current = newChild;
+  } else {
+// If we created `current` in the loop above, it was marked an
+// `INTERNAL` node. It should actually be an inactive leaf node.
+current->kind = Node::INACTIVE_LEAF;
   }
 
   // `current` is the newly created node associated with the last
   // element of the path. `current` should be an inactive node with no
   // children.
   CHECK(current->children.empty());
-  CHECK(!current->active);
+  CHECK(current->kind == Node::INACTIVE_LEAF);
 
   // Add a new entry to the lookup table. The full path of the newly
   // added client should not already exist in `clients`.
@@ -223,10 +229,12 @@ void DRFSorter::remove(const string& clientPath)
 
   if (child->name == ".") {
 CHECK(child->children.empty());
+CHECK(child->kind == Node::ACTIVE_LEAF ||
+  child->kind == Node::INACTIVE_LEAF);
 CHECK(clients.contains(current->path));
 CHECK_EQ(child, clients.at(current->path));
 
-current->active = child->active;
+current->kind = child->kind;
 current->removeChild(child);
 
 clients[current->path] = current;
@@ -250,14 +258,14 @@ void DRFSorter::remove(const string& clientPath)
 void 

mesos git commit: Fixed typos in test comments.

[neilc]

Repository: mesos
Updated Branches:
  refs/heads/master 43b33be03 -> 49129bf2a


Fixed typos in test comments.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/49129bf2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/49129bf2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/49129bf2

Branch: refs/heads/master
Commit: 49129bf2a6562c905f986b4ab7d3da9c8435dfa9
Parents: 43b33be
Author: Neil Conway 
Authored: Wed May 24 15:48:45 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 15:48:45 2017 -0700

--
 src/tests/containerizer/provisioner_appc_tests.cpp   | 4 ++--
 src/tests/containerizer/provisioner_docker_tests.cpp | 4 ++--
 src/tests/disk_quota_tests.cpp   | 2 +-
 src/tests/http_authentication_tests.cpp  | 2 +-
 src/tests/reservation_tests.cpp  | 2 +-
 src/tests/upgrade_tests.cpp  | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/49129bf2/src/tests/containerizer/provisioner_appc_tests.cpp
--
diff --git a/src/tests/containerizer/provisioner_appc_tests.cpp 
b/src/tests/containerizer/provisioner_appc_tests.cpp
index cba83c6..2af5fbd 100644
--- a/src/tests/containerizer/provisioner_appc_tests.cpp
+++ b/src/tests/containerizer/provisioner_appc_tests.cpp
@@ -84,7 +84,7 @@ namespace tests {
  * @param name Appc image name.
  * @param arch Machine architecture(e.g, x86, amd64).
  * @param os Operating system(e.g, linux).
- * @return Appc protobuff message object.
+ * @return Appc protobuf message object.
  */
 static Image::Appc getAppcImage(
 const string& name,
@@ -664,7 +664,7 @@ public:
   }
 
 private:
-  // TODO(jojy): Currently hard-codes the images dierctory name.
+  // TODO(jojy): Currently hard-codes the images directory name.
   // Consider parameterizing the directory name. This could be done
   // by removing the 'const' ness of the variable and adding mutator.
   const string imagesDirName;

http://git-wip-us.apache.org/repos/asf/mesos/blob/49129bf2/src/tests/containerizer/provisioner_docker_tests.cpp
--
diff --git a/src/tests/containerizer/provisioner_docker_tests.cpp 
b/src/tests/containerizer/provisioner_docker_tests.cpp
index 54b7fba..1decc72 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -305,7 +305,7 @@ public:
 };
 
 
-// This tests the store to pull the same image simutanuously.
+// This tests the store to pull the same image simultaneously.
 // This test verifies that the store only calls the puller once
 // when multiple requests for the same image is in flight.
 TEST_F(ProvisionerDockerLocalStoreTest, PullingSameImageSimutanuously)
@@ -917,7 +917,7 @@ TEST_F(ProvisionerDockerTest, 
ROOT_INTERNET_CURL_ImageDigest)
 
 // This test verifies that if a container image is specified, the
 // command runs as the specified user 'nobody' and the sandbox of
-// the command task is writtable by the specified user. It also
+// the command task is writeable by the specified user. It also
 // verifies that stdout/stderr are owned by the specified user.
 TEST_F(ProvisionerDockerTest, ROOT_INTERNET_CURL_CommandTaskUser)
 {

http://git-wip-us.apache.org/repos/asf/mesos/blob/49129bf2/src/tests/disk_quota_tests.cpp
--
diff --git a/src/tests/disk_quota_tests.cpp b/src/tests/disk_quota_tests.cpp
index 9fe2c86..a0a7093 100644
--- a/src/tests/disk_quota_tests.cpp
+++ b/src/tests/disk_quota_tests.cpp
@@ -291,7 +291,7 @@ TEST_F(DiskQuotaTest, VolumeUsageExceedsQuota)
 
   const Offer& offer = offers.get()[0];
 
-  // Create a task that requests a 1 MB persistent volume but atempts
+  // Create a task that requests a 1 MB persistent volume but attempts
   // to use 2MB.
   Resources volume = createPersistentVolume(
   Megabytes(1),

http://git-wip-us.apache.org/repos/asf/mesos/blob/49129bf2/src/tests/http_authentication_tests.cpp
--
diff --git a/src/tests/http_authentication_tests.cpp 
b/src/tests/http_authentication_tests.cpp
index 99bc257..f84af7c 100644
--- a/src/tests/http_authentication_tests.cpp
+++ b/src/tests/http_authentication_tests.cpp
@@ -342,7 +342,7 @@ AuthenticationResult createCombinedForbidden(
 //
 // Note: This test relies on the order of invocation of the installed
 // authenticators. If the `CombinedAuthenticator` is changed in the future to
-// call them in a different order, this test must be udpated.
+// call them in a different 

[3/7] mesos git commit: Added benchmark for allocator perf with many suppressed frameworks.

[neilc]

Added benchmark for allocator perf with many suppressed frameworks.

This covers the case where the vast majority (99%) of frameworks have
suppressed offers.

Review: https://reviews.apache.org/r/59383/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b7f33e47
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b7f33e47
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b7f33e47

Branch: refs/heads/master
Commit: b7f33e478793730d2334dbc92cff5c8ce558cfc7
Parents: 89e7bf5
Author: Neil Conway 
Authored: Tue May 23 10:36:14 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:40:13 2017 -0700

--
 src/tests/hierarchical_allocator_tests.cpp | 150 
 1 file changed, 150 insertions(+)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/b7f33e47/src/tests/hierarchical_allocator_tests.cpp
--
diff --git a/src/tests/hierarchical_allocator_tests.cpp 
b/src/tests/hierarchical_allocator_tests.cpp
index f90..7e5ade2 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -5501,6 +5501,156 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
SuppressOffers)
 }
 
 
+// This benchmark measures allocator performance when almost all
+// frameworks are suppressed.
+TEST_P(HierarchicalAllocator_BENCHMARK_Test, ExtremeSuppressOffers)
+{
+  size_t agentCount = std::get<0>(GetParam());
+  size_t frameworkCount = std::get<1>(GetParam());
+
+  // Pause the clock because we want to manually drive the allocations.
+  Clock::pause();
+
+  struct OfferedResources
+  {
+FrameworkID   frameworkId;
+SlaveID   slaveId;
+Resources resources;
+  };
+
+  vector offers;
+
+  auto offerCallback = [](
+  const FrameworkID& frameworkId,
+  const hashmap>& resources_)
+  {
+foreachkey (const string& role, resources_) {
+  foreachpair (const SlaveID& slaveId,
+   const Resources& resources,
+   resources_.at(role)) {
+offers.push_back(OfferedResources{frameworkId, slaveId, resources});
+  }
+}
+  };
+
+  cout << "Using " << agentCount << " agents and "
+   << frameworkCount << " frameworks" << endl;
+
+  master::Flags flags;
+  initialize(flags, offerCallback);
+
+  vector frameworks;
+  frameworks.reserve(frameworkCount);
+
+  Stopwatch watch;
+  watch.start();
+
+  for (size_t i = 0; i < frameworkCount; i++) {
+frameworks.push_back(createFrameworkInfo({"*"}));
+allocator->addFramework(frameworks[i].id(), frameworks[i], {}, true);
+  }
+
+  // Wait for all the `addFramework` operations to be processed.
+  Clock::settle();
+
+  watch.stop();
+
+  cout << "Added " << frameworkCount << " frameworks"
+   << " in " << watch.elapsed() << endl;
+
+  vector agents;
+  agents.reserve(agentCount);
+
+  const Resources agentResources = Resources::parse(
+  "cpus:24;mem:4096;disk:4096;ports:[31000-32000]").get();
+
+  // Each agent has a portion of its resources allocated to a single
+  // framework. We round-robin through the frameworks when allocating.
+  Resources allocation = Resources::parse("cpus:16;mem:1024;disk:1024").get();
+
+  Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 32000), 16);
+  ASSERT_SOME(ranges);
+  ASSERT_EQ(16, ranges->range_size());
+
+  allocation += createPorts(ranges.get());
+  allocation.allocate("*");
+
+  watch.start();
+
+  for (size_t i = 0; i < agentCount; i++) {
+agents.push_back(createSlaveInfo(agentResources));
+
+hashmap used;
+used[frameworks[i % frameworkCount].id()] = allocation;
+
+allocator->addSlave(
+agents[i].id(),
+agents[i],
+AGENT_CAPABILITIES(),
+None(),
+agents[i].resources(),
+used);
+  }
+
+  // Wait for all the `addSlave` operations to be processed.
+  Clock::settle();
+
+  watch.stop();
+
+  cout << "Added " << agentCount << " agents"
+   << " in " << watch.elapsed() << endl;
+
+  // Now perform allocations. Each time we trigger an allocation run, we
+  // increase the number of frameworks that are suppressing offers. To
+  // ensure the test can run in a timely manner, we always perform a
+  // fixed number of allocations.
+  //
+  // TODO(jjanco): Parameterize this test by allocationsCount, not an arbitrary
+  // number. Batching reduces loop size, lowering time to test completion.
+  size_t allocationsCount = 5;
+
+  // Suppress offers for 99% of frameworks.
+  size_t suppressCount = static_cast(frameworkCount * 0.99);
+  CHECK(suppressCount < frameworkCount);
+
+  for (size_t i = 0; i < suppressCount; i++) {
+

[2/7] mesos git commit: Added a new sorter test case, `HierarchicalIterationOrder`.

[neilc]

Added a new sorter test case, `HierarchicalIterationOrder`.

This behavior is covered by existing test cases to an extent, but some
additional test coverage seems warranted.

Review: https://reviews.apache.org/r/59482/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/89e7bf54
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/89e7bf54
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/89e7bf54

Branch: refs/heads/master
Commit: 89e7bf548c87c53a3e49fb6d930ed0d49d08affc
Parents: 7533901
Author: Neil Conway 
Authored: Tue May 23 10:36:13 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:40:08 2017 -0700

--
 src/tests/sorter_tests.cpp | 48 +
 1 file changed, 48 insertions(+)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/89e7bf54/src/tests/sorter_tests.cpp
--
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index ed66b9a..6ca724d 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -605,6 +605,54 @@ TEST(SorterTest, HierarchicalAllocation)
 }
 
 
+// This test checks that the sorted list of clients returned by the
+// sorter iterates over the client tree in the correct order.
+TEST(SorterTest, HierarchicalIterationOrder)
+{
+  DRFSorter sorter;
+
+  SlaveID slaveId;
+  slaveId.set_value("agentId");
+
+  Resources totalResources = Resources::parse("cpus:100;mem:100").get();
+  sorter.add(slaveId, totalResources);
+
+  sorter.add("a/b");
+  sorter.add("c");
+  sorter.add("d");
+  sorter.add("d/e");
+
+  sorter.activate("a/b");
+  sorter.activate("c");
+  sorter.activate("d");
+  sorter.activate("d/e");
+
+  // Shares: a/b = 0, c = 0, d = 0, d/e = 0
+  EXPECT_EQ(vector({"a/b", "c", "d", "d/e"}), sorter.sort());
+
+  Resources cResources = Resources::parse("cpus:8;mem:8").get();
+  sorter.allocated("c", slaveId, cResources);
+
+  // Shares: a/b = 0, d = 0, d/e = 0, c = 0.08.
+  EXPECT_EQ(vector({"a/b", "d", "d/e", "c"}), sorter.sort());
+
+  Resources dResources = Resources::parse("cpus:3;mem:3").get();
+  sorter.allocated("d", slaveId, dResources);
+
+  Resources deResources = Resources::parse("cpus:2;mem:2").get();
+  sorter.allocated("d/e", slaveId, deResources);
+
+  // Shares: a/b = 0, d/e = 0.02, d = 0.03, c = 0.08.
+  EXPECT_EQ(vector({"a/b", "d/e", "d", "c"}), sorter.sort());
+
+  Resources abResources = Resources::parse("cpus:6;mem:6").get();
+  sorter.allocated("a/b", slaveId, abResources);
+
+  // Shares: d/e = 0.02, d = 0.03, a/b = 0.06, c = 0.08.
+  EXPECT_EQ(vector({"d/e", "d", "a/b", "c"}), sorter.sort());
+}
+
+
 // This test checks what happens when a new sorter client is added as
 // a child of what was previously a leaf node.
 TEST(SorterTest, AddChildToLeaf)

[7/7] mesos git commit: Optimized sorter performance with many inactive clients.

[neilc]

Optimized sorter performance with many inactive clients.

Unlike in Mesos <= 1.2, the sorter now stores inactive clients in the
same data structure used to store active clients. This resulted in a
significant performance regression when the vast majority of sorter
clients are inactive: when sorting clients and producing the sorted
order, we iterate over ALL clients (active and inactive), which could be
much slower than the old active-only implementation.

This commit revises the sorter to ensure that inactive leaf nodes are
always stored at the end of their parent's list of child nodes. This
allows the sorter to stop early (at the first inactive leaf) when
iterating over a node's children, if we're only interested in applying
an operation to each active leaf or internal node. This change fixes the
observed performance regression relative to Mesos 1.2.0.

Review: https://reviews.apache.org/r/59355/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a637df6d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a637df6d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a637df6d

Branch: refs/heads/master
Commit: a637df6d3807ab13ce6f426a03cb4396ffa453e6
Parents: fd58fa1
Author: Neil Conway 
Authored: Tue May 23 10:36:18 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:40:39 2017 -0700

--
 src/master/allocator/sorter/drf/sorter.cpp | 98 +
 src/master/allocator/sorter/drf/sorter.hpp | 27 ++-
 2 files changed, 110 insertions(+), 15 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/a637df6d/src/master/allocator/sorter/drf/sorter.cpp
--
diff --git a/src/master/allocator/sorter/drf/sorter.cpp 
b/src/master/allocator/sorter/drf/sorter.cpp
index 1d8959d..ecc5586 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -147,6 +147,14 @@ void DRFSorter::add(const string& clientPath)
 // If we created `current` in the loop above, it was marked an
 // `INTERNAL` node. It should actually be an inactive leaf node.
 current->kind = Node::INACTIVE_LEAF;
+
+// `current` has changed from an internal node to an inactive
+// leaf, so remove and re-add it to its parent. This moves it to
+// the end of the parent's list of children.
+CHECK_NOTNULL(current->parent);
+
+current->parent->removeChild(current);
+current->parent->addChild(current);
   }
 
   // `current` is the newly created node associated with the last
@@ -229,6 +237,16 @@ void DRFSorter::remove(const string& clientPath)
 current->kind = child->kind;
 current->removeChild(child);
 
+// `current` has changed kind (from `INTERNAL` to a leaf,
+// which might be active or inactive). Hence we might need to
+// change its position in the `children` list.
+if (current->kind == Node::INTERNAL) {
+  CHECK_NOTNULL(current->parent);
+
+  current->parent->removeChild(current);
+  current->parent->addChild(current);
+}
+
 clients[current->path] = current;
 
 delete child;
@@ -250,14 +268,41 @@ void DRFSorter::remove(const string& clientPath)
 void DRFSorter::activate(const string& clientPath)
 {
   Node* client = CHECK_NOTNULL(find(clientPath));
-  client->kind = Node::ACTIVE_LEAF;
+
+  if (client->kind == Node::INACTIVE_LEAF) {
+client->kind = Node::ACTIVE_LEAF;
+
+// `client` has been activated, so move it to the beginning of its
+// parent's list of children. We mark the tree dirty, so that the
+// client's share is updated correctly and it is sorted properly.
+//
+// TODO(neilc): We could instead calculate share here and insert
+// the client into the appropriate place here, which would avoid
+// dirtying the whole tree.
+CHECK_NOTNULL(client->parent);
+
+client->parent->removeChild(client);
+client->parent->addChild(client);
+
+dirty = true;
+  }
 }
 
 
 void DRFSorter::deactivate(const string& clientPath)
 {
   Node* client = CHECK_NOTNULL(find(clientPath));
-  client->kind = Node::INACTIVE_LEAF;
+
+  if (client->kind == Node::ACTIVE_LEAF) {
+client->kind = Node::INACTIVE_LEAF;
+
+// `client` has been deactivated, so move it to the end of its
+// parent's list of children.
+CHECK_NOTNULL(client->parent);
+
+client->parent->removeChild(client);
+client->parent->addChild(client);
+  }
 }
 
 
@@ -467,16 +512,33 @@ vector DRFSorter::sort()
 {
   if (dirty) {
 std::function sortTree = [this, ](Node* node) {
-  foreach (Node* child, node->children) {
+  // Inactive leaves are always stored at the end of the
+  // `children` vector; 

[4/7] mesos git commit: Cleaned up allocator benchmark tests.

[neilc]

Cleaned up allocator benchmark tests.

Review: https://reviews.apache.org/r/59538


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8139ec0b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8139ec0b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8139ec0b

Branch: refs/heads/master
Commit: 8139ec0bd5c7daead3b7e4d05806898a48a6f62d
Parents: b7f33e4
Author: Neil Conway 
Authored: Wed May 24 13:19:55 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:40:21 2017 -0700

--
 src/tests/hierarchical_allocator_tests.cpp | 36 ++---
 1 file changed, 21 insertions(+), 15 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/8139ec0b/src/tests/hierarchical_allocator_tests.cpp
--
diff --git a/src/tests/hierarchical_allocator_tests.cpp 
b/src/tests/hierarchical_allocator_tests.cpp
index 7e5ade2..eb2b647 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -4971,9 +4971,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
AddAndUpdateSlave)
   // Add the slaves, use round-robin to choose which framework
   // to allocate a slice of the slave's resources to.
   for (size_t i = 0; i < slaves.size(); i++) {
-hashmap used;
-
-used[frameworks[i % frameworkCount].id()] = allocation;
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), allocation}
+};
 
 allocator->addSlave(
 slaves[i].id(),
@@ -5096,10 +5096,12 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
DeclineOffers)
   for (size_t i = 0; i < slaveCount; i++) {
 slaves.push_back(createSlaveInfo(agentResources));
 
-// Add some used resources on each slave. Let's say there are 16 tasks, 
each
-// is allocated 1 cpu and a random port from the port range.
-hashmap used;
-used[frameworks[i % frameworkCount].id()] = allocation;
+// Add some used resources on each slave. Let's say there are 16 tasks;
+// each is allocated 1 cpu and a random port from the port range.
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), allocation}
+};
+
 allocator->addSlave(
 slaves[i].id(),
 slaves[i],
@@ -5289,8 +5291,10 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
ResourceLabels)
 
 // Add some used resources on each slave. Let's say there are 16 tasks, 
each
 // is allocated 1 cpu and a random port from the port range.
-hashmap used;
-used[frameworks[i % frameworkCount].id()] = _allocation;
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), _allocation}
+};
+
 allocator->addSlave(
 slaves[i].id(),
 slaves[i],
@@ -5424,8 +5428,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
SuppressOffers)
   for (size_t i = 0; i < agentCount; i++) {
 agents.push_back(createSlaveInfo(agentResources));
 
-hashmap used;
-used[frameworks[i % frameworkCount].id()] = allocation;
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), allocation}
+};
 
 allocator->addSlave(
 agents[i].id(),
@@ -5454,7 +5459,7 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
SuppressOffers)
   size_t allocationsCount = 5;
   size_t suppressCount = 0;
 
-  for (size_t i = 0; i < allocationsCount; ++i) {
+  for (size_t i = 0; i < allocationsCount; i++) {
 // Recover resources with no filters because we want to test the
 // effect of suppression alone.
 foreach (const OfferedResources& offer, offers) {
@@ -5580,8 +5585,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
ExtremeSuppressOffers)
   for (size_t i = 0; i < agentCount; i++) {
 agents.push_back(createSlaveInfo(agentResources));
 
-hashmap used;
-used[frameworks[i % frameworkCount].id()] = allocation;
+hashmap used = {
+  {frameworks[i % frameworkCount].id(), allocation}
+};
 
 allocator->addSlave(
 agents[i].id(),
@@ -5617,7 +5623,7 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, 
ExtremeSuppressOffers)
 allocator->suppressOffers(frameworks[i].id(), {});
   }
 
-  for (size_t i = 0; i < allocationsCount; ++i) {
+  for (size_t i = 0; i < allocationsCount; i++) {
 // Recover resources with no filters because we want to test the
 // effect of suppression alone.
 foreach (const OfferedResources& offer, offers) {

[1/7] mesos git commit: Added sorter test for allocation queries about inactive clients.

[neilc]

Repository: mesos
Updated Branches:
  refs/heads/master 4faca73e9 -> a637df6d3


Added sorter test for allocation queries about inactive clients.

This behavior has not changed, but was not covered by the existing
tests.

Review: https://reviews.apache.org/r/59481/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7533901a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7533901a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7533901a

Branch: refs/heads/master
Commit: 7533901a3fd699adba7070e31d291026366e6431
Parents: 4faca73
Author: Neil Conway 
Authored: Tue May 23 10:36:12 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:39:58 2017 -0700

--
 src/tests/sorter_tests.cpp | 37 +
 1 file changed, 37 insertions(+)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/7533901a/src/tests/sorter_tests.cpp
--
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 2389664..ed66b9a 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -989,6 +989,43 @@ TEST(SorterTest, UpdateAllocationNestedClient)
 }
 
 
+// This test checks that the sorter correctly reports allocation
+// information about inactive clients.
+TEST(SorterTest, AllocationForInactiveClient)
+{
+  DRFSorter sorter;
+
+  SlaveID slaveId;
+  slaveId.set_value("agentId");
+
+  sorter.add(slaveId, Resources::parse("cpus:10;mem:10").get());
+
+  sorter.add("a");
+  sorter.add("b");
+
+  // Leave client "a" inactive.
+  sorter.activate("b");
+
+  sorter.allocated("a", slaveId, Resources::parse("cpus:2;mem:2").get());
+  sorter.allocated("b", slaveId, Resources::parse("cpus:3;mem:3").get());
+
+  hashmap clientAllocation = sorter.allocation(slaveId);
+  EXPECT_EQ(2u, clientAllocation.size());
+  EXPECT_EQ(Resources::parse("cpus:2;mem:2").get(), clientAllocation.at("a"));
+  EXPECT_EQ(Resources::parse("cpus:3;mem:3").get(), clientAllocation.at("b"));
+
+  hashmap agentAllocation1 = sorter.allocation("a");
+  EXPECT_EQ(1u, agentAllocation1.size());
+  EXPECT_EQ(
+  Resources::parse("cpus:2;mem:2").get(), agentAllocation1.at(slaveId));
+
+  hashmap agentAllocation2 = sorter.allocation("b");
+  EXPECT_EQ(1u, agentAllocation2.size());
+  EXPECT_EQ(
+  Resources::parse("cpus:3;mem:3").get(), agentAllocation2.at(slaveId));
+}
+
+
 // We aggregate resources from multiple slaves into the sorter.
 // Since non-scalar resources don't aggregate well across slaves,
 // we need to keep track of the SlaveIDs of the resources. This

[6/7] mesos git commit: Introduced `DRFSorter::Node::isLeaf()`.

[neilc]

Introduced `DRFSorter::Node::isLeaf()`.

This helps clarify code that wants to distinguish between leaves and
internal nodes in the sorter.

Review: https://reviews.apache.org/r/59484/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fd58fa14
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fd58fa14
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fd58fa14

Branch: refs/heads/master
Commit: fd58fa149fad807232eb68fed2a4fc22c8a08f54
Parents: dcb0b7c
Author: Neil Conway 
Authored: Tue May 23 10:36:17 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:40:34 2017 -0700

--
 src/master/allocator/sorter/drf/sorter.cpp | 22 ++
 src/master/allocator/sorter/drf/sorter.hpp | 10 ++
 2 files changed, 16 insertions(+), 16 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/fd58fa14/src/master/allocator/sorter/drf/sorter.cpp
--
diff --git a/src/master/allocator/sorter/drf/sorter.cpp 
b/src/master/allocator/sorter/drf/sorter.cpp
index 74da5b1..1d8959d 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -99,13 +99,7 @@ void DRFSorter::add(const string& clientPath)
 // leaf node into an internal node, we need to create an
 // additional child node: `current` must have been associated with
 // a client and clients must always be associated with leaf nodes.
-//
-// There are two exceptions: if `current` is the root node or it
-// was just created by the current `add()` call, it does not
-// correspond to a client, so we don't create an extra child.
-if (current->children.empty() &&
-current != root &&
-current != lastCreatedNode) {
+if (current->isLeaf()) {
   Node* parent = CHECK_NOTNULL(current->parent);
 
   parent->removeChild(current);
@@ -122,9 +116,10 @@ void DRFSorter::add(const string& clientPath)
   // `internal`.
   current->name = ".";
   current->parent = internal;
-  internal->addChild(current);
   current->path = strings::join("/", parent->path, current->name);
 
+  internal->addChild(current);
+
   CHECK_EQ(internal->path, current->clientPath());
 
   current = internal;
@@ -155,8 +150,7 @@ void DRFSorter::add(const string& clientPath)
   }
 
   // `current` is the newly created node associated with the last
-  // element of the path. `current` should be an inactive node with no
-  // children.
+  // element of the path. `current` should be an inactive leaf node.
   CHECK(current->children.empty());
   CHECK(current->kind == Node::INACTIVE_LEAF);
 
@@ -228,9 +222,7 @@ void DRFSorter::remove(const string& clientPath)
   Node* child = *(current->children.begin());
 
   if (child->name == ".") {
-CHECK(child->children.empty());
-CHECK(child->kind == Node::ACTIVE_LEAF ||
-  child->kind == Node::INACTIVE_LEAF);
+CHECK(child->isLeaf());
 CHECK(clients.contains(current->path));
 CHECK_EQ(child, clients.at(current->path));
 
@@ -578,9 +570,7 @@ DRFSorter::Node* DRFSorter::find(const string& clientPath) 
const
 
   Node* client = client_.get();
 
-  CHECK(client->kind == Node::ACTIVE_LEAF ||
-client->kind == Node::INACTIVE_LEAF);
-  CHECK(client->children.empty());
+  CHECK(client->isLeaf());
 
   return client;
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/fd58fa14/src/master/allocator/sorter/drf/sorter.hpp
--
diff --git a/src/master/allocator/sorter/drf/sorter.hpp 
b/src/master/allocator/sorter/drf/sorter.hpp
index f76d2f7..67700d5 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -267,6 +267,16 @@ struct DRFSorter::Node
 return path;
   }
 
+  bool isLeaf() const
+  {
+if (kind == ACTIVE_LEAF || kind == INACTIVE_LEAF) {
+  CHECK(children.empty());
+  return true;
+}
+
+return false;
+  }
+
   void removeChild(const Node* child)
   {
 auto it = std::find(children.begin(), children.end(), child);

[5/7] mesos git commit: Replaced the sorter's notion of "activation" with a three-valued enum.

[neilc]

Replaced the sorter's notion of "activation" with a three-valued enum.

Conceptually, a node in the sorter tree can either be an internal node
or a leaf node. Furthermore, leaf nodes can either be active or inactive
(internal nodes do not have a concept of "activation").

We previously represented this situation with a single boolean,
`active`. The boolean was true for active leaves, and false for inactive
leaves and internal nodes. Whether a node was an internal node was
determined by checking the number of children it has.

This commit replaces `active` with a three-valued enum named `kind`,
which can take on the values `ACTIVE_LEAF`, `INACTIVE_LEAF`, or
`INTERNAL`. This enforces the idea that internal nodes do not have a
notion of "activation", and more explicitly distinguishes between leaf
and internal nodes.

Review: https://reviews.apache.org/r/59483/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/dcb0b7ce
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/dcb0b7ce
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/dcb0b7ce

Branch: refs/heads/master
Commit: dcb0b7ce95309be64886c7ee36413c3665cfef9b
Parents: 8139ec0
Author: Neil Conway 
Authored: Tue May 23 10:36:16 2017 -0700
Committer: Neil Conway 
Committed: Wed May 24 14:40:28 2017 -0700

--
 src/master/allocator/sorter/drf/sorter.cpp | 40 +
 src/master/allocator/sorter/drf/sorter.hpp | 22 +-
 2 files changed, 42 insertions(+), 20 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/dcb0b7ce/src/master/allocator/sorter/drf/sorter.cpp
--
diff --git a/src/master/allocator/sorter/drf/sorter.cpp 
b/src/master/allocator/sorter/drf/sorter.cpp
index 26b77f5..74da5b1 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -45,13 +45,13 @@ namespace allocator {
 
 
 DRFSorter::DRFSorter()
-  : root(new Node("", nullptr)) {}
+  : root(new Node("", Node::INTERNAL, nullptr)) {}
 
 
 DRFSorter::DRFSorter(
 const UPID& allocator,
 const string& metricsPrefix)
-  : root(new Node("", nullptr)),
+  : root(new Node("", Node::INTERNAL, nullptr)),
 metrics(Metrics(allocator, *this, metricsPrefix)) {}
 
 
@@ -112,7 +112,7 @@ void DRFSorter::add(const string& clientPath)
 
   // Create a node under `parent`. This internal node will take
   // the place of `current` in the tree.
-  Node* internal = new Node(current->name, parent);
+  Node* internal = new Node(current->name, Node::INTERNAL, parent);
   parent->addChild(internal);
   internal->allocation = current->allocation;
 
@@ -131,28 +131,34 @@ void DRFSorter::add(const string& clientPath)
 }
 
 // Now actually add a new child to `current`.
-Node* newChild = new Node(element, current);
+Node* newChild = new Node(element, Node::INTERNAL, current);
 current->addChild(newChild);
 
 current = newChild;
 lastCreatedNode = newChild;
   }
 
+  CHECK(current->kind == Node::INTERNAL);
+
   // `current` is the node associated with the last element of the
   // path. If we didn't add `current` to the tree above, create a leaf
   // node now. For example, if the tree contains "a/b" and we add a
   // new client "a", we want to create a new leaf node "a/." here.
   if (current != lastCreatedNode) {
-Node* newChild = new Node(".", current);
+Node* newChild = new Node(".", Node::INACTIVE_LEAF, current);
 current->addChild(newChild);
 current = newChild;
+  } else {
+// If we created `current` in the loop above, it was marked an
+// `INTERNAL` node. It should actually be an inactive leaf node.
+current->kind = Node::INACTIVE_LEAF;
   }
 
   // `current` is the newly created node associated with the last
   // element of the path. `current` should be an inactive node with no
   // children.
   CHECK(current->children.empty());
-  CHECK(!current->active);
+  CHECK(current->kind == Node::INACTIVE_LEAF);
 
   // Add a new entry to the lookup table. The full path of the newly
   // added client should not already exist in `clients`.
@@ -223,10 +229,12 @@ void DRFSorter::remove(const string& clientPath)
 
   if (child->name == ".") {
 CHECK(child->children.empty());
+CHECK(child->kind == Node::ACTIVE_LEAF ||
+  child->kind == Node::INACTIVE_LEAF);
 CHECK(clients.contains(current->path));
 CHECK_EQ(child, clients.at(current->path));
 
-current->active = child->active;
+current->kind = child->kind;
 current->removeChild(child);
 
 clients[current->path] = current;
@@ -250,14 +258,14 @@ void DRFSorter::remove(const string& clientPath)
 void