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 20:30:19 UTC

[mesos] branch master updated (7a52eef -> 38e41c5)

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

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


    from 7a52eef  Added MESOS-9555 to the 1.5.3 CHANGELOG.
     new e10034f  Fixed variable names in `Master::_doRegistryGC()`.
     new d794ab7  Removed unreachable tasks from `Master::Framework` on agent GC.
     new 38e41c5  Tested unreachable task behavior on agent GC.

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/master/master.cpp         | 44 +++++++++++++++++++++++++++++++------------
 src/tests/partition_tests.cpp | 40 +++++++++++++++++++++++++++++++--------
 2 files changed, 64 insertions(+), 20 deletions(-)


[mesos] 01/03: 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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit e10034fb4fede1aaf58b25c7e9fe3e677826350b
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 8e23785..4870227 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1796,7 +1796,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.
@@ -1804,7 +1804,7 @@ void Master::doRegistryGc()
 
       size_t liveCount = count - toRemove.size();
       if (liveCount > flags.registry_max_agent_count) {
-        toRemove.insert(slave);
+        toRemove.insert(slaveId);
         continue;
       }
 
@@ -1813,7 +1813,7 @@ void Master::doRegistryGc()
           currentTime.nanoseconds() - removalTime.nanoseconds());
 
       if (age > flags.registry_max_agent_age) {
-        toRemove.insert(slave);
+        toRemove.insert(slaveId);
       }
     }
 
@@ -1866,29 +1866,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] 03/03: 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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 38e41c5fe4a3cd41b89961b8e3d4e03f2f35a6c3
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 15af47b..6cdfe973 100644
--- a/src/tests/partition_tests.cpp
+++ b/src/tests/partition_tests.cpp
@@ -2932,16 +2932,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());
@@ -2973,6 +2987,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`.
@@ -3031,13 +3047,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.
 
@@ -3052,15 +3068,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.
 
@@ -3094,7 +3118,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"]);


[mesos] 02/03: 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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit d794ab792f102aa6642b66b7b5dd9520ef283d94
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 4870227..106d924 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1875,7 +1875,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++;
   }