spark git commit: Revert "[SPARK-23020][CORE] Fix races in launcher code, test."

2018-01-16 Thread sameerag 
Repository: spark
Updated Branches:
  refs/heads/branch-2.3 0a441d2ed -> b9339eee1


Revert "[SPARK-23020][CORE] Fix races in launcher code, test."

This reverts commit 20c69816a63071b82b1035d4b48798c358206421.


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

Branch: refs/heads/branch-2.3
Commit: b9339eee1304c0309be4ea74f8cdc3d37a8048d3
Parents: 0a441d2e
Author: Sameer Agarwal 
Authored: Tue Jan 16 22:17:37 2018 -0800
Committer: Sameer Agarwal 
Committed: Tue Jan 16 22:17:37 2018 -0800

--
 .../spark/launcher/SparkLauncherSuite.java  | 49 +++-
 .../spark/launcher/AbstractAppHandle.java   | 22 ++---
 .../spark/launcher/ChildProcAppHandle.java  | 18 ---
 .../spark/launcher/InProcessAppHandle.java  | 17 ---
 .../spark/launcher/LauncherConnection.java  | 14 +++---
 .../apache/spark/launcher/LauncherServer.java   | 46 +++---
 .../org/apache/spark/launcher/BaseSuite.java| 42 +++--
 .../spark/launcher/LauncherServerSuite.java | 20 +---
 8 files changed, 72 insertions(+), 156 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/b9339eee/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java
--
diff --git 
a/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java 
b/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java
index a042375..9d2f563 100644
--- a/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java
+++ b/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java
@@ -17,7 +17,6 @@
 
 package org.apache.spark.launcher;
 
-import java.time.Duration;
 import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -32,7 +31,6 @@ import static org.junit.Assume.*;
 import static org.mockito.Mockito.*;
 
 import org.apache.spark.SparkContext;
-import org.apache.spark.SparkContext$;
 import org.apache.spark.internal.config.package$;
 import org.apache.spark.util.Utils;
 
@@ -139,9 +137,7 @@ public class SparkLauncherSuite extends BaseSuite {
   // Here DAGScheduler is stopped, while SparkContext.clearActiveContext 
may not be called yet.
   // Wait for a reasonable amount of time to avoid creating two active 
SparkContext in JVM.
   // See SPARK-23019 and SparkContext.stop() for details.
-  eventually(Duration.ofSeconds(5), Duration.ofMillis(10), () -> {
-assertTrue("SparkContext is still alive.", 
SparkContext$.MODULE$.getActive().isEmpty());
-  });
+  TimeUnit.MILLISECONDS.sleep(500);
 }
   }
 
@@ -150,35 +146,26 @@ public class SparkLauncherSuite extends BaseSuite {
 SparkAppHandle.Listener listener = mock(SparkAppHandle.Listener.class);
 doAnswer(invocation -> {
   SparkAppHandle h = (SparkAppHandle) invocation.getArguments()[0];
-  synchronized (transitions) {
-transitions.add(h.getState());
-  }
+  transitions.add(h.getState());
   return null;
 }).when(listener).stateChanged(any(SparkAppHandle.class));
 
-SparkAppHandle handle = null;
-try {
-  handle = new InProcessLauncher()
-.setMaster("local")
-.setAppResource(SparkLauncher.NO_RESOURCE)
-.setMainClass(InProcessTestApp.class.getName())
-.addAppArgs("hello")
-.startApplication(listener);
-
-  waitFor(handle);
-  assertEquals(SparkAppHandle.State.FINISHED, handle.getState());
-
-  // Matches the behavior of LocalSchedulerBackend.
-  List expected = Arrays.asList(
-SparkAppHandle.State.CONNECTED,
-SparkAppHandle.State.RUNNING,
-SparkAppHandle.State.FINISHED);
-  assertEquals(expected, transitions);
-} finally {
-  if (handle != null) {
-handle.kill();
-  }
-}
+SparkAppHandle handle = new InProcessLauncher()
+  .setMaster("local")
+  .setAppResource(SparkLauncher.NO_RESOURCE)
+  .setMainClass(InProcessTestApp.class.getName())
+  .addAppArgs("hello")
+  .startApplication(listener);
+
+waitFor(handle);
+assertEquals(SparkAppHandle.State.FINISHED, handle.getState());
+
+// Matches the behavior of LocalSchedulerBackend.
+List expected = Arrays.asList(
+  SparkAppHandle.State.CONNECTED,
+  SparkAppHandle.State.RUNNING,
+  SparkAppHandle.State.FINISHED);
+assertEquals(expected, transitions);
   }
 
   public static class SparkLauncherTestApp {

http://git-wip-us.apache.org/repos/asf/spark/blob/b9339eee/launcher/src/main/java/org/apache/spark/launcher/AbstractAppHandle.java

spark git commit: Revert "[SPARK-23020][CORE] Fix races in launcher code, test."

2018-01-16 Thread sameerag 
Repository: spark
Updated Branches:
  refs/heads/master 166705785 -> 50345a2aa


Revert "[SPARK-23020][CORE] Fix races in launcher code, test."

This reverts commit 66217dac4f8952a9923625908ad3dcb030763c81.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/50345a2a
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/50345a2a
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/50345a2a

Branch: refs/heads/master
Commit: 50345a2aa59741c511d555edbbad2da9611e7d16
Parents: 1667057
Author: Sameer Agarwal 
Authored: Tue Jan 16 22:14:47 2018 -0800
Committer: Sameer Agarwal 
Committed: Tue Jan 16 22:14:47 2018 -0800

--
 .../spark/launcher/SparkLauncherSuite.java  | 49 +++-
 .../spark/launcher/AbstractAppHandle.java   | 22 ++---
 .../spark/launcher/ChildProcAppHandle.java  | 18 ---
 .../spark/launcher/InProcessAppHandle.java  | 17 ---
 .../spark/launcher/LauncherConnection.java  | 14 +++---
 .../apache/spark/launcher/LauncherServer.java   | 46 +++---
 .../org/apache/spark/launcher/BaseSuite.java| 42 +++--
 .../spark/launcher/LauncherServerSuite.java | 20 +---
 8 files changed, 72 insertions(+), 156 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/50345a2a/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java
--
diff --git 
a/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java 
b/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java
index a042375..9d2f563 100644
--- a/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java
+++ b/core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java
@@ -17,7 +17,6 @@
 
 package org.apache.spark.launcher;
 
-import java.time.Duration;
 import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -32,7 +31,6 @@ import static org.junit.Assume.*;
 import static org.mockito.Mockito.*;
 
 import org.apache.spark.SparkContext;
-import org.apache.spark.SparkContext$;
 import org.apache.spark.internal.config.package$;
 import org.apache.spark.util.Utils;
 
@@ -139,9 +137,7 @@ public class SparkLauncherSuite extends BaseSuite {
   // Here DAGScheduler is stopped, while SparkContext.clearActiveContext 
may not be called yet.
   // Wait for a reasonable amount of time to avoid creating two active 
SparkContext in JVM.
   // See SPARK-23019 and SparkContext.stop() for details.
-  eventually(Duration.ofSeconds(5), Duration.ofMillis(10), () -> {
-assertTrue("SparkContext is still alive.", 
SparkContext$.MODULE$.getActive().isEmpty());
-  });
+  TimeUnit.MILLISECONDS.sleep(500);
 }
   }
 
@@ -150,35 +146,26 @@ public class SparkLauncherSuite extends BaseSuite {
 SparkAppHandle.Listener listener = mock(SparkAppHandle.Listener.class);
 doAnswer(invocation -> {
   SparkAppHandle h = (SparkAppHandle) invocation.getArguments()[0];
-  synchronized (transitions) {
-transitions.add(h.getState());
-  }
+  transitions.add(h.getState());
   return null;
 }).when(listener).stateChanged(any(SparkAppHandle.class));
 
-SparkAppHandle handle = null;
-try {
-  handle = new InProcessLauncher()
-.setMaster("local")
-.setAppResource(SparkLauncher.NO_RESOURCE)
-.setMainClass(InProcessTestApp.class.getName())
-.addAppArgs("hello")
-.startApplication(listener);
-
-  waitFor(handle);
-  assertEquals(SparkAppHandle.State.FINISHED, handle.getState());
-
-  // Matches the behavior of LocalSchedulerBackend.
-  List expected = Arrays.asList(
-SparkAppHandle.State.CONNECTED,
-SparkAppHandle.State.RUNNING,
-SparkAppHandle.State.FINISHED);
-  assertEquals(expected, transitions);
-} finally {
-  if (handle != null) {
-handle.kill();
-  }
-}
+SparkAppHandle handle = new InProcessLauncher()
+  .setMaster("local")
+  .setAppResource(SparkLauncher.NO_RESOURCE)
+  .setMainClass(InProcessTestApp.class.getName())
+  .addAppArgs("hello")
+  .startApplication(listener);
+
+waitFor(handle);
+assertEquals(SparkAppHandle.State.FINISHED, handle.getState());
+
+// Matches the behavior of LocalSchedulerBackend.
+List expected = Arrays.asList(
+  SparkAppHandle.State.CONNECTED,
+  SparkAppHandle.State.RUNNING,
+  SparkAppHandle.State.FINISHED);
+assertEquals(expected, transitions);
   }
 
   public static class SparkLauncherTestApp {

http://git-wip-us.apache.org/repos/asf/spark/blob/50345a2a/launcher/src/main/java/org/apache/spark/launcher/AbstractAppHandle.java