Repository: zeppelin
Updated Branches:
  refs/heads/master e763b3bf3 -> 65e1d3645


[ZEPPELIN-525] Test failing in zeppelin-interpreter

`RemoteSchedulerTest.test` was fall when CPU switched `RemoteScheduler` 
execution (which executes a job in a separate thread) back to test main thread 
execution after changing job status to `Status.FINISHED` but before updating 
running jobs list:
```
job.setStatus(lastStatus);

// if thread execution is switched here to the main thread test will fall

synchronized (queue) {
    running.remove(job);
    queue.notify();
}
```
Test checked job status, saw that job was terminated and expected to see an 
empty jobs runing list.
```
while (!job.isTerminated() && cycles < MAX_WAIT_CYCLES) {
    Thread.sleep(TICK_WAIT);
    cycles++;
}

// this assert will fail because a job was not removed from running list yet
assertEquals(0, scheduler.getJobsRunning().size());
```
But `scheduler.running` list still contained the last job and the assertion 
failed.

### What is this PR for?
The goal is to synchronize updating of a job status and a scheduler running 
jobs list when a job is terminated.

### What type of PR is it?
Bug Fix

### What is the Jira issue?
[ZEPPELIN-525](https://issues.apache.org/jira/browse/ZEPPELIN-525)

### How should this be tested?
You may place a `Thread.sleep(1000);` code to the `RemoteScheduler.java` class 
after the `job.setStatus(lastStatus);` (line number 354). This will increase 
probability of threads switching after this line. Test should not fail.

### Questions:
* Does the licenses files need update? **no**
* Is there breaking changes for older versions? **no**
* Does this needs documentation? **no**

Author: Alexander Shoshin <alexander_shos...@epam.com>

Closes #1954 from AlexanderShoshin/ZEPPELIN-525 and squashes the following 
commits:

8a0f4af [Alexander Shoshin] synchronized a job status and a list of running jobs


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/65e1d364
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/65e1d364
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/65e1d364

Branch: refs/heads/master
Commit: 65e1d364583ac8fb822aa00ebee5b7cfcd842fed
Parents: e763b3b
Author: Alexander Shoshin <alexander_shos...@epam.com>
Authored: Fri Jan 27 11:30:11 2017 +0300
Committer: Felix Cheung <felixche...@apache.org>
Committed: Sun Jan 29 23:35:53 2017 -0800

----------------------------------------------------------------------
 .../zeppelin/scheduler/RemoteScheduler.java     | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/65e1d364/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
index 0101b18..a4ab00e 100644
--- 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
+++ 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
@@ -307,10 +307,10 @@ public class RemoteScheduler implements Scheduler {
     @Override
     public void run() {
       if (job.isAborted()) {
-        job.setStatus(Status.ABORT);
-        job.aborted = false;
-
         synchronized (queue) {
+          job.setStatus(Status.ABORT);
+          job.aborted = false;
+
           running.remove(job);
           queue.notify();
         }
@@ -350,16 +350,16 @@ public class RemoteScheduler implements Scheduler {
         lastStatus = Status.ERROR;
       }
 
-      job.setStatus(lastStatus);
+      synchronized (queue) {
+        job.setStatus(lastStatus);
 
-      if (listener != null) {
-        listener.jobFinished(scheduler, job);
-      }
+        if (listener != null) {
+          listener.jobFinished(scheduler, job);
+        }
 
-      // reset aborted flag to allow retry
-      job.aborted = false;
+        // reset aborted flag to allow retry
+        job.aborted = false;
 
-      synchronized (queue) {
         running.remove(job);
         queue.notify();
       }

Reply via email to