You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2019/02/15 22:36:17 UTC

[mesos] branch 1.7.x updated (d018bab -> 6fcf701)

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

vinodkone pushed a change to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from d018bab  Added MESOS-9555 to the 1.7.2 CHANGELOG.
     new f0cd3b7  Fixed variable names in `Master::_doRegistryGC()`.
     new c72a4f9  Removed unreachable tasks from `Master::Framework` on agent GC.
     new 1a506a4  Tested unreachable task behavior on agent GC.
     new 6fcf701  Added MESOS-8887 to the 1.7.2 CHANGELOG.

The 4 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:
 CHANGELOG                     |  1 +
 src/master/master.cpp         | 44 +++++++++++++++++++++++++++++++------------
 src/tests/partition_tests.cpp | 40 +++++++++++++++++++++++++++++++--------
 3 files changed, 65 insertions(+), 20 deletions(-)


[mesos] 01/04: Fixed variable names in `Master::_doRegistryGC()`.

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

vinodkone pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit f0cd3b7b62807fe377b1b47bf1bf364b18c4a373
Author: Vinod Kone <vi...@gmail.com>
AuthorDate: Sat Feb 2 09:51:09 2019 -0600

    Fixed variable names in `Master::_doRegistryGC()`.
    
    Substituted `slave` with `slaveId` to be consistent with the code base.
    No functional changes.
    
    Review: https://reviews.apache.org/r/69907
---
 src/master/master.cpp | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/master/master.cpp b/src/master/master.cpp
index 873d685..354a512 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1820,7 +1820,7 @@ void Master::doRegistryGc()
     TimeInfo currentTime = protobuf::getCurrentTime();
     hashset<SlaveID> toRemove;
 
-    foreachpair (const SlaveID& slave,
+    foreachpair (const SlaveID& slaveId,
                  const TimeInfo& removalTime,
                  slaves) {
       // Count-based GC.
@@ -1828,7 +1828,7 @@ void Master::doRegistryGc()
 
       size_t liveCount = count - toRemove.size();
       if (liveCount > flags.registry_max_agent_count) {
-        toRemove.insert(slave);
+        toRemove.insert(slaveId);
         continue;
       }
 
@@ -1837,7 +1837,7 @@ void Master::doRegistryGc()
           currentTime.nanoseconds() - removalTime.nanoseconds());
 
       if (age > flags.registry_max_agent_age) {
-        toRemove.insert(slave);
+        toRemove.insert(slaveId);
       }
     }
 
@@ -1890,29 +1890,29 @@ void Master::_doRegistryGc(
   // operation, but there isn't an easy way to do that.
 
   size_t numRemovedUnreachable = 0;
-  foreach (const SlaveID& slave, toRemoveUnreachable) {
-    if (!slaves.unreachable.contains(slave)) {
-      LOG(WARNING) << "Failed to garbage collect " << slave
+  foreach (const SlaveID& slaveId, toRemoveUnreachable) {
+    if (!slaves.unreachable.contains(slaveId)) {
+      LOG(WARNING) << "Failed to garbage collect " << slaveId
                    << " from the unreachable list";
 
       continue;
     }
 
-    slaves.unreachable.erase(slave);
-    slaves.unreachableTasks.erase(slave);
+    slaves.unreachable.erase(slaveId);
+    slaves.unreachableTasks.erase(slaveId);
     numRemovedUnreachable++;
   }
 
   size_t numRemovedGone = 0;
-  foreach (const SlaveID& slave, toRemoveGone) {
-    if (!slaves.gone.contains(slave)) {
-      LOG(WARNING) << "Failed to garbage collect " << slave
+  foreach (const SlaveID& slaveId, toRemoveGone) {
+    if (!slaves.gone.contains(slaveId)) {
+      LOG(WARNING) << "Failed to garbage collect " << slaveId
                    << " from the gone list";
 
       continue;
     }
 
-    slaves.gone.erase(slave);
+    slaves.gone.erase(slaveId);
     numRemovedGone++;
   }
 


[mesos] 04/04: Added MESOS-8887 to the 1.7.2 CHANGELOG.

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

vinodkone pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 6fcf70167076bbe6fb10ca04876939fe0e3379d9
Author: Vinod Kone <vi...@gmail.com>
AuthorDate: Fri Feb 15 14:33:00 2019 -0600

    Added MESOS-8887 to the 1.7.2 CHANGELOG.
---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index 7cd5bf6..922e750 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,7 @@ Release Notes - Mesos - Version 1.7.2 (WIP)
 * This is a bug fix release.
 
 ** Bug
+  * [MESOS-8887] - Unreachable tasks are not GC'ed when unreachable agent is GC'ed.
   * [MESOS-9210] - Mesos v1 scheduler library does not properly handle SUBSCRIBE retries.
   * [MESOS-9517] - SLRP should treat gRPC timeouts as non-terminal errors, instead of reporting OPERATION_FAILED.
   * [MESOS-9531] - chown error handling is incorrect in createSandboxDirectory.


[mesos] 02/04: Removed unreachable tasks from `Master::Framework` on agent GC.

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

vinodkone pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit c72a4f909054e5efa75d9e5d8dde71b0083402c1
Author: Vinod Kone <vi...@gmail.com>
AuthorDate: Sat Feb 2 10:01:56 2019 -0600

    Removed unreachable tasks from `Master::Framework` on agent GC.
    
    Unreachable tasks are stored in `Slaves` and `Framework` structs of
    the master, but they were only being removed from the former when
    an unreachable agent is GCed from the registry. This patch fixes it
    so that the latter is also cleaned up.
    
    Review: https://reviews.apache.org/r/69908
---
 src/master/master.cpp | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/master/master.cpp b/src/master/master.cpp
index 354a512..a6bc23f 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1899,7 +1899,27 @@ void Master::_doRegistryGc(
     }
 
     slaves.unreachable.erase(slaveId);
+
+    // TODO(vinod): Consider moving these tasks into `completedTasks` by
+    // transitioning them to a terminal state and sending status updates.
+    // But it's not clear what this state should be. If a framework
+    // reconciles these tasks after this point it would get `TASK_UNKNOWN`
+    // which seems appropriate but we don't keep tasks in this state in-memory.
+    if (slaves.unreachableTasks.contains(slaveId)) {
+      foreachkey (const FrameworkID& frameworkId,
+                  slaves.unreachableTasks.at(slaveId)) {
+        Framework* framework = getFramework(frameworkId);
+        if (framework != nullptr) {
+          foreach (const TaskID& taskId,
+                   slaves.unreachableTasks.at(slaveId).get(frameworkId)) {
+            framework->unreachableTasks.erase(taskId);
+          }
+        }
+      }
+    }
+
     slaves.unreachableTasks.erase(slaveId);
+
     numRemovedUnreachable++;
   }
 


[mesos] 03/04: Tested unreachable task behavior on agent GC.

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

vinodkone pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 1a506a4536a4b79dba6634d8dc627eaf2a55caba
Author: Vinod Kone <vi...@gmail.com>
AuthorDate: Tue Feb 5 16:55:19 2019 -0600

    Tested unreachable task behavior on agent GC.
    
    Updated `PartitionTest, RegistryGcByCount` test. This test fails
    without the previous patch.
    
    Review: https://reviews.apache.org/r/69909
---
 src/tests/partition_tests.cpp | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/src/tests/partition_tests.cpp b/src/tests/partition_tests.cpp
index 788d3b9..38ea2d2 100644
--- a/src/tests/partition_tests.cpp
+++ b/src/tests/partition_tests.cpp
@@ -2929,16 +2929,30 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByCount)
 
   EXPECT_CALL(sched, registered(&driver, _, _));
 
-  Future<Nothing> resourceOffers;
+  Future<vector<Offer>> offers;
   EXPECT_CALL(sched, resourceOffers(&driver, _))
-    .WillOnce(FutureSatisfy(&resourceOffers))
+    .WillOnce(FutureArg<1>(&offers))
     .WillRepeatedly(Return()); // Ignore subsequent offers.
 
   driver.start();
 
   // Need to make sure the framework AND slave have registered with
   // master. Waiting for resource offers should accomplish both.
-  AWAIT_READY(resourceOffers);
+  AWAIT_READY(offers);
+
+  TaskInfo task = createTask(offers->at(0), SLEEP_COMMAND(60));
+
+  Future<TaskStatus> unreachableStatus;
+  EXPECT_CALL(sched, statusUpdate(&driver, _))
+    .WillOnce(FutureArg<1>(&unreachableStatus));
+
+  // Drop RunTaskMessage so that the task stays in TASK_STAGING.
+  Future<RunTaskMessage> runTaskMessage =
+    DROP_PROTOBUF(RunTaskMessage(), _, _);
+
+  driver.launchTasks(offers->at(0).id(), {task});
+
+  AWAIT_READY(runTaskMessage);
 
   EXPECT_CALL(sched, offerRescinded(&driver, _))
     .WillRepeatedly(Return());
@@ -2970,6 +2984,8 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByCount)
 
   Clock::advance(Milliseconds(100));
 
+  AWAIT_READY(unreachableStatus);
+
   AWAIT_READY(slaveLost1);
 
   // Set the expectation before resetting `slave1`.
@@ -3028,13 +3044,13 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByCount)
   slave2.get()->terminate();
   slave2->reset();
 
-  // Do explicit reconciliation for a random task ID on `slave1`. GC
+  // Do explicit reconciliation for the task launched on `slave1`. GC
   // has not occurred yet (since `registry_gc_interval` has not
   // elapsed since the master was started), so the slave should be in
   // the unreachable list; hence `unreachable_time` should be set on
   // the result of the reconciliation request.
   TaskStatus status1;
-  status1.mutable_task_id()->set_value(id::UUID::random().toString());
+  status1.mutable_task_id()->CopyFrom(task.task_id());
   status1.mutable_slave_id()->CopyFrom(slaveId1);
   status1.set_state(TASK_STAGING); // Dummy value.
 
@@ -3049,15 +3065,23 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByCount)
   EXPECT_EQ(TaskStatus::REASON_RECONCILIATION, reconcileUpdate1->reason());
   EXPECT_EQ(partitionTime1, reconcileUpdate1->unreachable_time());
 
+  JSON::Object stats = Metrics();
+  EXPECT_EQ(1, stats.values["master/tasks_unreachable"]);
+
   // Advance the clock to cause GC to be performed.
   Clock::advance(masterFlags.registry_gc_interval);
   Clock::settle();
 
-  // Do explicit reconciliation for a random task ID on `slave1`.
+  // Ensure task has been removed from unreachable list once the
+  // agent is GC'ed.
+  stats = Metrics();
+  EXPECT_EQ(0, stats.values["master/tasks_unreachable"]);
+
+  // Do explicit reconciliation for the task launched on `slave1`.
   // Because the agent has been removed from the unreachable list in
   // the registry, `unreachable_time` should NOT be set.
   TaskStatus status2;
-  status2.mutable_task_id()->set_value(id::UUID::random().toString());
+  status2.mutable_task_id()->CopyFrom(task.task_id());
   status2.mutable_slave_id()->CopyFrom(slaveId1);
   status2.set_state(TASK_STAGING); // Dummy value.
 
@@ -3091,7 +3115,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByCount)
   EXPECT_EQ(TaskStatus::REASON_RECONCILIATION, reconcileUpdate3->reason());
   EXPECT_EQ(partitionTime2, reconcileUpdate3->unreachable_time());
 
-  JSON::Object stats = Metrics();
+  stats = Metrics();
   EXPECT_EQ(2, stats.values["master/slave_unreachable_scheduled"]);
   EXPECT_EQ(2, stats.values["master/slave_unreachable_completed"]);
   EXPECT_EQ(2, stats.values["master/slave_removals"]);