You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2018/10/30 19:19:25 UTC

[mesos] branch master updated (81564b8 -> c1c8665)

This is an automated email from the ASF dual-hosted git repository.

josephwu pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from 81564b8  Fixed a typo in `slave.cpp`.
     new b6e00dd  Removed extraneous 'virtual' keyword.
     new a2d6a81  Fixed LongLivedDefaultExecutorRestart GC test.
     new c1c8665  Fixed FetcherTest.DuplicateFileURI on OSX.

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/common/type_utils.cpp                |  6 +++++-
 src/examples/inverse_offer_framework.cpp |  2 +-
 src/tests/gc_tests.cpp                   | 13 +++++++++----
 src/v1/mesos.cpp                         |  6 +++++-
 4 files changed, 20 insertions(+), 7 deletions(-)


[mesos] 02/03: Fixed LongLivedDefaultExecutorRestart GC test.

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

josephwu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit a2d6a81f5dc47967726a6205799825b345231e80
Author: Joseph Wu <jo...@apache.org>
AuthorDate: Mon Oct 29 12:07:20 2018 -0700

    Fixed LongLivedDefaultExecutorRestart GC test.
    
    This test was incorrectly restarting the agent actor inside the test.
    In the flaky test, the agent actor would be started with an auto-
    generated PID (i.e. `slave(1)`, `slave(2)`, etc).  Because of how this
    generation works, each PID will be unique.  The executor in the test
    would be launched under `slave(1)` but the restarted agent would have
    a PID of `slave(2)`.  This meant the executor's reregistration would
    fail with '404 Not Found' and the executor would be cleaned up.
    
    The executor cleanup would potentially trigger a TASK_LOST status
    update; and if that update is sent prior to ending the test, this
    will break some mock expectations and cause the test to fail.
    
    This changes the test to always use the same PID for the agent actor.
    
    Review: https://reviews.apache.org/r/69203
---
 src/tests/gc_tests.cpp | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/tests/gc_tests.cpp b/src/tests/gc_tests.cpp
index 4d94430..a583f1d 100644
--- a/src/tests/gc_tests.cpp
+++ b/src/tests/gc_tests.cpp
@@ -903,8 +903,7 @@ TEST_F(GarbageCollectorIntegrationTest, LongLivedDefaultExecutor)
 // when a task finishes, but the executor is still running. This version of
 // the test restarts the agent to ensure recovered tasks are also scheduled
 // for GC.
-TEST_F(
-    GarbageCollectorIntegrationTest, DISABLED_LongLivedDefaultExecutorRestart)
+TEST_F(GarbageCollectorIntegrationTest, LongLivedDefaultExecutorRestart)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
@@ -915,8 +914,14 @@ TEST_F(
   // Turn on GC of nested container sandboxes by default.
   flags.gc_non_executor_container_sandboxes = true;
 
+  // Start the slave with a static process ID. This allows the executor to
+  // reconnect with the slave upon a process restart.
+  const string id(process::ID::generate("agent"));
+
   Owned<MasterDetector> detector = master.get()->createDetector();
-  Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+  Try<Owned<cluster::Slave>> slave =
+    StartSlave(detector.get(), id, flags, false);
+
   ASSERT_SOME(slave);
 
   auto scheduler = std::make_shared<v1::MockHTTPScheduler>();
@@ -1094,7 +1099,7 @@ TEST_F(
 
   // The agent should reregister once recovery is complete, which also means
   // that any finished tasks metadata/sandboxes should be rescheduled for GC.
-  slave = StartSlave(detector.get(), flags);
+  slave = StartSlave(detector.get(), id, flags, false);
   ASSERT_SOME(slave);
   AWAIT_READY(slaveReregisteredMessage);
 


[mesos] 03/03: Fixed FetcherTest.DuplicateFileURI on OSX.

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

josephwu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit c1c866529cf53356b3fdbdb102c008f63ba40c61
Author: Joseph Wu <jo...@apache.org>
AuthorDate: Mon Oct 29 16:00:06 2018 -0700

    Fixed FetcherTest.DuplicateFileURI on OSX.
    
    This test seems to fail because of a mismatch between the implementation
    of std::hash and operator== of the CommandInfo::URI object.  The hash
    accounts for all fields except for 'cache'.  The equality operator
    accounts for all fields except 'cache' **and** 'output_file'.
    
    This mismatch results in replacing the URI value in the fetcher's
    mapping, rather than adding a value.  Why the std::unordered_map
    checks the equality operator rather than the hash value is unknown,
    but may be an implementation detail of clang's stdlib.
    
    Review: https://reviews.apache.org/r/69205
---
 src/common/type_utils.cpp | 6 +++++-
 src/v1/mesos.cpp          | 6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/common/type_utils.cpp b/src/common/type_utils.cpp
index 33d6380..ef13eae 100644
--- a/src/common/type_utils.cpp
+++ b/src/common/type_utils.cpp
@@ -80,9 +80,13 @@ bool operator==(const CommandInfo& left, const CommandInfo& right)
 
 bool operator==(const CommandInfo::URI& left, const CommandInfo::URI& right)
 {
+  // NOTE: We purposefully do not compare the value of the `cache` field
+  // because a URI downloaded from source or from the fetcher cache should
+  // be considered identical.
   return left.value() == right.value() &&
     left.executable() == right.executable() &&
-    left.extract() == right.extract();
+    left.extract() == right.extract() &&
+    left.output_file() == right.output_file();
 }
 
 
diff --git a/src/v1/mesos.cpp b/src/v1/mesos.cpp
index 9b2df2d..704ad76 100644
--- a/src/v1/mesos.cpp
+++ b/src/v1/mesos.cpp
@@ -79,9 +79,13 @@ bool operator==(const CommandInfo& left, const CommandInfo& right)
 
 bool operator==(const CommandInfo::URI& left, const CommandInfo::URI& right)
 {
+  // NOTE: We purposefully do not compare the value of the `cache` field
+  // because a URI downloaded from source or from the fetcher cache should
+  // be considered identical.
   return left.value() == right.value() &&
     left.executable() == right.executable() &&
-    left.extract() == right.extract();
+    left.extract() == right.extract() &&
+    left.output_file() == right.output_file();
 }
 
 


[mesos] 01/03: Removed extraneous 'virtual' keyword.

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

josephwu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit b6e00ddf9cdc97fd0d8c27f2d337ef4c77d2b550
Author: Joseph Wu <jo...@apache.org>
AuthorDate: Mon Oct 29 16:06:55 2018 -0700

    Removed extraneous 'virtual' keyword.
---
 src/examples/inverse_offer_framework.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/examples/inverse_offer_framework.cpp b/src/examples/inverse_offer_framework.cpp
index 0e43492..360d6c6 100644
--- a/src/examples/inverse_offer_framework.cpp
+++ b/src/examples/inverse_offer_framework.cpp
@@ -110,7 +110,7 @@ public:
   ~InverseOfferScheduler() override {}
 
 protected:
-  virtual void initialize() override
+  void initialize() override
   {
     // We initialize the library here to ensure that callbacks are only invoked
     // after the process has spawned.