[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mesos-commits
Subject:    [1/2] git commit: Refactored the scheduler/executor driver to remove calling 'stop'.
From:       benh () apache ! org
Date:       2014-06-29 16:28:19
Message-ID: 96befbe71e5746f0bfd28df9ce246b84 () git ! apache ! org
[Download RAW message or body]

Repository: mesos
Updated Branches:
  refs/heads/master 451c3b6a7 -> 294337466


Refactored the scheduler/executor driver to remove calling 'stop'.

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


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

Branch: refs/heads/master
Commit: 0821ffeca1ec958c1bb01b203d57d20de3933341
Parents: 451c3b6
Author: Benjamin Hindman <benjamin.hindman@gmail.com>
Authored: Fri Jun 27 12:07:30 2014 -0700
Committer: Benjamin Hindman <benjamin.hindman@gmail.com>
Committed: Sun Jun 29 09:25:14 2014 -0700

----------------------------------------------------------------------
 src/exec/exec.cpp                                      |  6 ++++--
 src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp  | 10 ++++++----
 src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp | 10 ++++++----
 src/python/native/mesos_executor_driver_impl.cpp       |  2 --
 src/python/native/mesos_scheduler_driver_impl.cpp      |  2 --
 5 files changed, 16 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0821ffec/src/exec/exec.cpp
----------------------------------------------------------------------
diff --git a/src/exec/exec.cpp b/src/exec/exec.cpp
index 346e39f..e09ac8d 100644
--- a/src/exec/exec.cpp
+++ b/src/exec/exec.cpp
@@ -598,8 +598,10 @@ MesosExecutorDriver::MesosExecutorDriver(Executor* _executor)
 
 MesosExecutorDriver::~MesosExecutorDriver()
 {
-  // Just as in SchedulerProcess, we might wait here indefinitely if
-  // MesosExecutorDriver::stop has not been invoked.
+  // Just like with the MesosSchedulerDriver it's possible to get a
+  // deadlock here. Otherwise we terminate the ExecutorProcess and
+  // wait for it before deleting.
+  terminate(process);
   wait(process);
   delete process;
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/0821ffec/src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp
                
----------------------------------------------------------------------
diff --git a/src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp \
b/src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp index 32e9a28..8cbc798 100644
--- a/src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp
+++ b/src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp
@@ -390,10 +390,12 @@ JNIEXPORT void JNICALL \
Java_org_apache_mesos_MesosExecutorDriver_finalize  MesosExecutorDriver* driver =
     (MesosExecutorDriver*) env->GetLongField(thiz, __driver);
 
-  // Call stop just in case.
-  driver->stop();
-  driver->join();
-
+  // Note that we DO NOT want to call 'abort' or 'stop' as this may be
+  // misinterpreted by the executor. It is possible, however, that
+  // since we haven't called 'abort' or 'stop' there are still threads
+  // executing within the executor callbacks but the
+  // MesosExecutorDriver destructor will wait until this is not the
+  // case before returning.
   delete driver;
 
   jfieldID __executor = env->GetFieldID(clazz, "__executor", "J");

http://git-wip-us.apache.org/repos/asf/mesos/blob/0821ffec/src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp
                
----------------------------------------------------------------------
diff --git a/src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp \
b/src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp index d0b7152..3498930 \
                100644
--- a/src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp
+++ b/src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp
@@ -548,10 +548,12 @@ JNIEXPORT void JNICALL \
Java_org_apache_mesos_MesosSchedulerDriver_finalize  MesosSchedulerDriver* driver =
     (MesosSchedulerDriver*) env->GetLongField(thiz, __driver);
 
-  // Call stop just in case.
-  driver->stop();
-  driver->join();
-
+  // Note that we DO NOT want to call 'abort' or 'stop' as this may be
+  // misinterpreted by the scheduler. It is possible, however, that
+  // since we haven't called 'abort' or 'stop' there are still threads
+  // executing within the scheduler callbacks but the
+  // MesosSchedulerDriver destructor will wait until this is not the
+  // case before returning.
   delete driver;
 
   jfieldID __scheduler = env->GetFieldID(clazz, "__scheduler", "J");

http://git-wip-us.apache.org/repos/asf/mesos/blob/0821ffec/src/python/native/mesos_executor_driver_impl.cpp
                
----------------------------------------------------------------------
diff --git a/src/python/native/mesos_executor_driver_impl.cpp \
b/src/python/native/mesos_executor_driver_impl.cpp index aa564ed..407d8d1 100644
--- a/src/python/native/mesos_executor_driver_impl.cpp
+++ b/src/python/native/mesos_executor_driver_impl.cpp
@@ -167,7 +167,6 @@ int MesosExecutorDriverImpl_init(MesosExecutorDriverImpl *self,
   }
 
   if (self->driver != NULL) {
-    self->driver->stop();
     delete self->driver;
     self->driver = NULL;
   }
@@ -190,7 +189,6 @@ int MesosExecutorDriverImpl_init(MesosExecutorDriverImpl *self,
 void MesosExecutorDriverImpl_dealloc(MesosExecutorDriverImpl* self)
 {
   if (self->driver != NULL) {
-    self->driver->stop();
     // We need to wrap the driver destructor in an "allow threads"
     // macro since the MesosExecutorDriver destructor waits for the
     // ExecutorProcess to terminate and there might be a thread that

http://git-wip-us.apache.org/repos/asf/mesos/blob/0821ffec/src/python/native/mesos_scheduler_driver_impl.cpp
                
----------------------------------------------------------------------
diff --git a/src/python/native/mesos_scheduler_driver_impl.cpp \
b/src/python/native/mesos_scheduler_driver_impl.cpp index 1c82532..e014eed 100644
--- a/src/python/native/mesos_scheduler_driver_impl.cpp
+++ b/src/python/native/mesos_scheduler_driver_impl.cpp
@@ -214,7 +214,6 @@ int MesosSchedulerDriverImpl_init(MesosSchedulerDriverImpl* self,
 
 
   if (self->driver != NULL) {
-    self->driver->stop();
     delete self->driver;
     self->driver = NULL;
   }
@@ -244,7 +243,6 @@ int MesosSchedulerDriverImpl_init(MesosSchedulerDriverImpl* self,
 void MesosSchedulerDriverImpl_dealloc(MesosSchedulerDriverImpl* self)
 {
   if (self->driver != NULL) {
-    self->driver->stop();
     // We need to wrap the driver destructor in an "allow threads"
     // macro since the MesosSchedulerDriver destructor waits for the
     // SchedulerProcess to terminate and there might be a thread that


[prev in list] [next in list] [prev in thread] [next in thread]