Repository: reef
Updated Branches:
  refs/heads/master e4a2a075f -> 485242c57


[REEF-1728] Improve logging in the Driver when Evaluator is being closed.

Better wording in error and log messages during the Evaluator closure
process on the Driver side. No changes in functionality in this PR.

JIRA:
  [REEF-1728](https://issues.apache.org/jira/browse/REEF-1728)

Pull request:
  This closes #1244


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

Branch: refs/heads/master
Commit: 485242c5781afec1e0cab9d0303efa74133c9542
Parents: e4a2a07
Author: Sergiy Matusevych <[email protected]>
Authored: Thu Jan 26 13:06:07 2017 -0800
Committer: Mariia Mykhailova <[email protected]>
Committed: Thu Jan 26 15:44:41 2017 -0800

----------------------------------------------------------------------
 .../driver/evaluator/EvaluatorManager.java      |  1 +
 .../common/driver/evaluator/Evaluators.java     | 11 +++----
 .../resourcemanager/ResourceStatusHandler.java  | 34 ++++++++++++--------
 3 files changed, 27 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/reef/blob/485242c5/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
 
b/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
index 26af25f..61564b1 100644
--- 
a/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
+++ 
b/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
@@ -285,6 +285,7 @@ public final class EvaluatorManager implements 
Identifiable, AutoCloseable {
    * Close message dispatcher for the evaluator.
    */
   public void shutdown() {
+    LOG.log(Level.FINEST, "Shutdown EvaluatorManager: {0}", this.evaluatorId);
     this.messageDispatcher.close();
   }
 

http://git-wip-us.apache.org/repos/asf/reef/blob/485242c5/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/Evaluators.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/Evaluators.java
 
b/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/Evaluators.java
index 2e6974b..13894b7 100644
--- 
a/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/Evaluators.java
+++ 
b/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/evaluator/Evaluators.java
@@ -156,26 +156,25 @@ public final class Evaluators implements AutoCloseable {
   public synchronized void removeClosedEvaluator(final EvaluatorManager 
evaluatorManager) {
 
     final String evaluatorId = evaluatorManager.getId();
+    LOG.log(Level.FINE, "Removing closed evaluator: {0}", evaluatorId);
 
     if (!evaluatorManager.isClosed()) {
-      throw new IllegalArgumentException("Trying to remove evaluator " + 
evaluatorId + " which is not closed yet.");
+      throw new IllegalArgumentException("Removing evaluator that has not been 
closed yet: " + evaluatorId);
     }
 
     if (!this.evaluators.containsKey(evaluatorId) && 
!this.closedEvaluatorIds.contains(evaluatorId)) {
-      throw new IllegalArgumentException("Trying to remove unknown evaluator " 
+ evaluatorId + ".");
+      throw new IllegalArgumentException("Removing unknown evaluator: " + 
evaluatorId);
     }
 
     if (!this.evaluators.containsKey(evaluatorId) && 
this.closedEvaluatorIds.contains(evaluatorId)) {
-      LOG.log(Level.FINE, "Trying to remove closed evaluator {0} which has 
already been removed.", evaluatorId);
+      LOG.log(Level.FINE, "Removing closed evaluator which has already been 
removed: {0}", evaluatorId);
       return;
     }
 
-    LOG.log(Level.FINE, "Removing closed evaluator {0}", evaluatorId);
-
     evaluatorManager.shutdown();
     this.evaluators.remove(evaluatorId);
     this.closedEvaluatorIds.add(evaluatorId);
 
-    LOG.log(Level.FINEST, "Evaluator {0} removed", evaluatorId);
+    LOG.log(Level.FINEST, "Closed evaluator removed: {0}", evaluatorId);
   }
 }

http://git-wip-us.apache.org/repos/asf/reef/blob/485242c5/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/resourcemanager/ResourceStatusHandler.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/resourcemanager/ResourceStatusHandler.java
 
b/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/resourcemanager/ResourceStatusHandler.java
index 50ee841..b1529de 100644
--- 
a/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/resourcemanager/ResourceStatusHandler.java
+++ 
b/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/driver/resourcemanager/ResourceStatusHandler.java
@@ -61,30 +61,38 @@ public final class ResourceStatusHandler implements 
EventHandler<ResourceStatusE
    */
   @Override
   public void onNext(final ResourceStatusEvent resourceStatusEvent) {
-    final Optional<EvaluatorManager> evaluatorManager = 
this.evaluators.get(resourceStatusEvent.getIdentifier());
-    if (evaluatorManager.isPresent()) {
-      evaluatorManager.get().onResourceStatusMessage(resourceStatusEvent);
 
-      if (evaluatorManager.get().isClosed()) {
-        this.evaluators.removeClosedEvaluator(evaluatorManager.get());
+    final String id = resourceStatusEvent.getIdentifier();
+    final Optional<EvaluatorManager> evaluatorManager = 
this.evaluators.get(id);
+
+    LOG.log(Level.FINEST, "Evaluator {0} status: {1}",
+        new Object[] {evaluatorManager, resourceStatusEvent.getState()});
+
+    if (evaluatorManager.isPresent()) {
+      final EvaluatorManager evaluatorManagerImpl = evaluatorManager.get();
+      evaluatorManagerImpl.onResourceStatusMessage(resourceStatusEvent);
+      if (evaluatorManagerImpl.isClosed()) {
+        this.evaluators.removeClosedEvaluator(evaluatorManagerImpl);
       }
+
     } else {
-      if (this.evaluators.wasClosed(resourceStatusEvent.getIdentifier())) {
-        LOG.log(Level.WARNING, "Unexpected resource status from closed 
evaluator " +
-            resourceStatusEvent.getIdentifier() + " with state " + 
resourceStatusEvent.getState());
+
+      if (this.evaluators.wasClosed(id)) {
+        LOG.log(Level.WARNING,
+            "Unexpected resource status from closed evaluator {0} with state 
{1}",
+            new Object[] {id, resourceStatusEvent.getState()});
       }
 
-      if 
(driverRestartManager.get().getEvaluatorRestartState(resourceStatusEvent.getIdentifier())
-            .isFailedOrExpired()) {
+      if 
(driverRestartManager.get().getEvaluatorRestartState(id).isFailedOrExpired()) {
+
         final EvaluatorManager previousEvaluatorManager = 
this.evaluatorManagerFactory
             
.getNewEvaluatorManagerForEvaluatorFailedDuringDriverRestart(resourceStatusEvent);
 
         previousEvaluatorManager.onResourceStatusMessage(resourceStatusEvent);
+
       } else {
         throw new RuntimeException(
-            "Unknown resource status from evaluator " + 
resourceStatusEvent.getIdentifier() +
-                " with state " + resourceStatusEvent.getState()
-        );
+            "Unknown resource status from evaluator " + id + " with state " + 
resourceStatusEvent.getState());
       }
     }
   }

Reply via email to