[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2017-06-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r122050475
  
--- Diff: 
core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java ---
@@ -183,12 +185,38 @@ public void testChildProcLauncher() throws Exception {
 assertEquals(0, app.waitFor());
   }
 
+  @Test
+  public void testThreadLauncher() throws Exception {
+// This test is failed on Windows due to the failure of initiating 
executors
+// by the path length limitation. See SPARK-18718.
+assumeTrue(!Utils.isWindows());
+
+launcher = new SparkLauncher();
+launcher
+  .setMaster("local")
+  .setAppResource(SparkLauncher.NO_RESOURCE)
+  .setConf(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS,
+"-Dfoo=bar -Dtest.appender=childproc")
+  .setConf(SparkLauncher.DRIVER_EXTRA_CLASSPATH, 
System.getProperty("java.class.path"))
+  .setMainClass(SparkLauncherTestApp.class.getName())
+  .launchAsThread(true)
+  .addAppArgs("thread");
+final SparkAppHandle app = launcher.startApplication();
+sleep(3000);
+AbstractSparkAppHandle handle = (AbstractSparkAppHandle)app;
+handle.waitFor();
--- End diff --

In this unit test the main application is Simple application. There is no 
spark application running in background to track status, so no way to use 
`handle.getState`. All I am checking is for launched thread/process is complete.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2017-06-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r122049171
  
--- Diff: 
core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java ---
@@ -183,12 +185,38 @@ public void testChildProcLauncher() throws Exception {
 assertEquals(0, app.waitFor());
   }
 
+  @Test
+  public void testThreadLauncher() throws Exception {
+// This test is failed on Windows due to the failure of initiating 
executors
+// by the path length limitation. See SPARK-18718.
+assumeTrue(!Utils.isWindows());
+
+launcher = new SparkLauncher();
+launcher
+  .setMaster("local")
+  .setAppResource(SparkLauncher.NO_RESOURCE)
+  .setConf(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS,
+"-Dfoo=bar -Dtest.appender=childproc")
+  .setConf(SparkLauncher.DRIVER_EXTRA_CLASSPATH, 
System.getProperty("java.class.path"))
+  .setMainClass(SparkLauncherTestApp.class.getName())
+  .launchAsThread(true)
+  .addAppArgs("thread");
+final SparkAppHandle app = launcher.startApplication();
+sleep(3000);
+AbstractSparkAppHandle handle = (AbstractSparkAppHandle)app;
--- End diff --

`waitFor` is a package private now. This method is added for unit test in 
`AbstractSparkAppHandle` not part of the interface `SparkAppHandle`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-05-24 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin I added `waitFor` method to ChildSparkHandle allowing it to wait 
for launched process or thread. Now the test waits for thread to finish.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-05-17 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin I modified the test with sleep for SparkApp to run.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2017-05-09 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r115573484
  
--- Diff: 
core/src/test/java/org/apache/spark/launcher/SparkLauncherSuite.java ---
@@ -183,6 +183,26 @@ public void testChildProcLauncher() throws Exception {
 assertEquals(0, app.waitFor());
   }
 
+  @Test
+  public void testThreadLauncher() throws Exception {
+// This test is failed on Windows due to the failure of initiating 
executors
+// by the path length limitation. See SPARK-18718.
+assumeTrue(!Utils.isWindows());
+
+launcher = new SparkLauncher();
+launcher
+  .setMaster("local")
+  .setAppResource(SparkLauncher.NO_RESOURCE)
+  .setConf(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS,
+"-Dfoo=bar -Dtest.appender=childproc")
+  .setConf(SparkLauncher.DRIVER_EXTRA_CLASSPATH, 
System.getProperty("java.class.path"))
+  .setMainClass(SparkLauncherTestApp.class.getName())
+  .launchAsThread(true)
+  .addAppArgs("proc");
+final SparkAppHandle app = launcher.startApplication();
+assertEquals(false, app.getState().isFinal());
--- End diff --

@vanzin, I am looking into it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-04-07 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin @tgravescs,
I removed `envvars` argument from SparkApp and all unwanted code along with 
it.
It still provides autoShutdown in both Thread mode and process mode for 
yarn cluster mode Spark jobs. I tested all test scenarios mentioned at the top.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-03-24 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin , Please review and suggest if you have any further changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-03-22 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin, 
- I refactored unit tests
- Changed LAUNCHER_INTERNAL* variable access to package private
- Fixed some of the nits.

About passing sys.env, we want to capture set of sys.env variables while 
launching the new SparkApp. In thread mode, the launching user code could 
change the sys.env variables after launching application. For instance even 
pollute env while launching another Spark app.  We made a lot of code changes 
to ensure this.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2017-03-22 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r107498853
  
--- Diff: 
resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala
 ---
@@ -201,6 +210,152 @@ class YarnClusterSuite extends BaseYarnClusterSuite {
 finalState should be (SparkAppHandle.State.FAILED)
   }
 
+  test("monitor app using launcher library for thread") {
+val env = new JHashMap[String, String]()
+env.put("YARN_CONF_DIR", hadoopConfDir.getAbsolutePath())
+
+val propsFile = createConfFile()
+val handle = new SparkLauncher(env)
+  .setSparkHome(sys.props("spark.test.home"))
+  .setConf("spark.ui.enabled", "false")
+  .setPropertiesFile(propsFile)
+  .setMaster("yarn")
+  .setDeployMode("cluster")
+  .launchAsThread(true)
+  .setAppResource(SparkLauncher.NO_RESOURCE)
+  .setMainClass(mainClassName(YarnLauncherTestApp.getClass))
+  .startApplication()
+
+try {
+  eventually(timeout(30 seconds), interval(100 millis)) {
+handle.getState() should be (SparkAppHandle.State.RUNNING)
+  }
+
+  handle.getAppId() should not be (null)
+  handle.getAppId() should startWith ("application_")
+  handle.stop()
+
+  eventually(timeout(30 seconds), interval(100 millis)) {
+handle.getState() should be (SparkAppHandle.State.KILLED)
+  }
+} finally {
+  handle.kill()
+}
+  }
+
+  test("monitor app using launcher library for proc with auto shutdown") {
+val env = new JHashMap[String, String]()
+env.put("YARN_CONF_DIR", hadoopConfDir.getAbsolutePath())
+
+val propsFile = createConfFile()
+val handle = new SparkLauncher(env)
+  .setSparkHome(sys.props("spark.test.home"))
+  .setConf("spark.ui.enabled", "false")
+  .setPropertiesFile(propsFile)
+  .setMaster("yarn")
+  .setDeployMode("cluster")
+  .autoShutdown()
+  .setAppResource(SparkLauncher.NO_RESOURCE)
+  .setMainClass(mainClassName(YarnLauncherTestApp.getClass))
+  .startApplication()
+
+try {
+  eventually(timeout(30 seconds), interval(100 millis)) {
+handle.getState() should be (SparkAppHandle.State.RUNNING)
+  }
+
+  handle.getAppId() should not be (null)
+  handle.getAppId() should startWith ("application_")
+  handle.disconnect()
+  val applicationId = ConverterUtils.toApplicationId(handle.getAppId)
+  val yarnClient: YarnClient = getYarnClient
+  eventually(timeout(30 seconds), interval(100 millis)) {
+handle.getState() should be (SparkAppHandle.State.LOST)
+var status = 
yarnClient.getApplicationReport(applicationId).getFinalApplicationStatus()
+status should be (FinalApplicationStatus.KILLED)
+  }
+
+} finally {
+  handle.kill()
+}
+  }
+
+  test("monitor app using launcher library for thread with auto shutdown") 
{
--- End diff --

refactored


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2017-03-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r106080005
  
--- Diff: 
resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala
 ---
@@ -252,20 +307,55 @@ class YarnClusterSuite extends BaseYarnClusterSuite {
   handle.getAppId() should startWith ("application_")
   handle.disconnect()
 
+  var applicationId = ConverterUtils.toApplicationId(handle.getAppId)
+  var yarnClient: YarnClient = getYarnClient
   eventually(timeout(30 seconds), interval(100 millis)) {
 handle.getState() should be (SparkAppHandle.State.LOST)
+var status = 
yarnClient.getApplicationReport(applicationId).getFinalApplicationStatus
+status should be (FinalApplicationStatus.KILLED)
   }
+
 } finally {
   handle.kill()
 }
   }
 
-  test("timeout to get SparkContext in cluster mode triggers failure") {
-val timeout = 2000
-val finalState = runSpark(false, 
mainClassName(SparkContextTimeoutApp.getClass),
-  appArgs = Seq((timeout * 4).toString),
-  extraConf = Map(AM_MAX_WAIT_TIME.key -> timeout.toString))
-finalState should be (SparkAppHandle.State.FAILED)
+  test("monitor app using launcher library for thread without auto 
shutdown") {
+val env = new JHashMap[String, String]()
+env.put("YARN_CONF_DIR", hadoopConfDir.getAbsolutePath())
+
+val propsFile = createConfFile()
+val handle = new SparkLauncher(env)
+  .setSparkHome(sys.props("spark.test.home"))
+  .setConf("spark.ui.enabled", "false")
+  .setPropertiesFile(propsFile)
+  .setMaster("yarn")
+  .setDeployMode("cluster")
+  .launchAsThread(true)
+  .setAppResource(SparkLauncher.NO_RESOURCE)
+  .setMainClass(mainClassName(YarnLauncherTestApp.getClass))
+  .startApplication()
+
+try {
+  eventually(timeout(30 seconds), interval(100 millis)) {
+handle.getState() should be (SparkAppHandle.State.RUNNING)
+  }
+
+  handle.getAppId() should not be (null)
+  handle.getAppId() should startWith ("application_")
+  handle.disconnect()
+
+  var applicationId = ConverterUtils.toApplicationId(handle.getAppId)
+  var yarnClient: YarnClient = getYarnClient
+  eventually(timeout(30 seconds), interval(100 millis)) {
+handle.getState() should be (SparkAppHandle.State.LOST)
+var status = 
yarnClient.getApplicationReport(applicationId).getYarnApplicationState
+status should not be (YarnApplicationState.KILLED)
--- End diff --

Checking condition status should `not` be. It could be running of 
successful but not killed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-03-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@tgravescs, I added few more tests to cover `autoStudown` options impact on 
launching it as thread and proc. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-03-08 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
The build failure seems unrelated.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-03-08 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin @tgravescs, I have updates the  patch with suggested changes. 
 - Removed unused `connect` method from `LauncherBackend`.
 - Documented expected behavior for `SparkAppHandler#disconnect` along with 
`autoShutdown`.
 - Added unit test and integration tests for thread and autoShutdown.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2017-02-07 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin,
I have updated the patch to address the review comments. Sorry about 
confusion on props and environment variables. I have separated that as argument 
in SparkApp method.  Also, simplified and cleaned up other places in the code 
such as `ChildProcAppHandle`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2017-02-07 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r99850931
  
--- Diff: 
core/src/main/scala/org/apache/spark/launcher/LauncherBackend.scala ---
@@ -17,38 +17,67 @@
 
 package org.apache.spark.launcher
 
+import java.io.IOException
 import java.net.{InetAddress, Socket}
 
 import org.apache.spark.SPARK_VERSION
+import org.apache.spark.internal.Logging
 import org.apache.spark.launcher.LauncherProtocol._
-import org.apache.spark.util.{ThreadUtils, Utils}
+import org.apache.spark.util.{ShutdownHookManager, ThreadUtils, Utils}
 
 /**
  * A class that can be used to talk to a launcher server. Users should 
extend this class to
  * provide implementation for the abstract methods.
  *
  * See `LauncherServer` for an explanation of how launcher communication 
works.
  */
-private[spark] abstract class LauncherBackend {
+private[spark] abstract class LauncherBackend extends Logging {
 
   private var clientThread: Thread = _
   private var connection: BackendConnection = _
   private var lastState: SparkAppHandle.State = _
+  private var stopOnShutdown: Boolean = false
   @volatile private var _isConnected = false
 
   def connect(): Unit = {
 val port = sys.env.get(LauncherProtocol.ENV_LAUNCHER_PORT).map(_.toInt)
 val secret = sys.env.get(LauncherProtocol.ENV_LAUNCHER_SECRET)
+val stopFlag = 
sys.env.get(LauncherProtocol.ENV_LAUNCHER_STOP_FLAG).map(_.toBoolean)
 if (port != None && secret != None) {
-  val s = new Socket(InetAddress.getLoopbackAddress(), port.get)
-  connection = new BackendConnection(s)
-  connection.send(new Hello(secret.get, SPARK_VERSION))
-  clientThread = LauncherBackend.threadFactory.newThread(connection)
-  clientThread.start()
-  _isConnected = true
+  connect(port.get, secret.get, stopFlag.getOrElse(false))
 }
   }
 
+  def connect(port: Int, secret: String): Unit = {
+val s = new Socket(InetAddress.getLoopbackAddress(), port)
+connection = new BackendConnection(s)
+connection.send(new Hello(secret, SPARK_VERSION))
+clientThread = LauncherBackend.threadFactory.newThread(connection)
+clientThread.start()
+_isConnected = true
+if (stopOnShutdown) {
+  logDebug("Adding shutdown hook") // force eager creation of logger
+  var _shutdownHookRef = ShutdownHookManager.addShutdownHook(
+ShutdownHookManager.SPARK_CONTEXT_SHUTDOWN_PRIORITY) { () =>
+logInfo("Invoking onStopRequest() from shutdown hook")
+try {
+  if (_isConnected && stopOnShutdown) {
+onStopRequest()
+  }
+}
+catch {
+  case anotherIOE: IOException =>
+logError("Error while running LauncherBackend 
shutdownHook...", anotherIOE)
+}
+  }
+}
+  }
+
+  def connect(port: Int, secret: String, stopFlag: Boolean): Unit = {
--- End diff --

This addressed now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2017-02-07 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r99849329
  
--- Diff: 
core/src/main/scala/org/apache/spark/launcher/LauncherBackend.scala ---
@@ -71,6 +100,9 @@ private[spark] abstract class LauncherBackend {
 if (connection != null && lastState != state) {
   connection.send(new SetState(state))
   lastState = state
+  if (!_isConnected && stopOnShutdown) {
--- End diff --

The `LauncherBackend` is supposed to run within `Spark App` JVM.  The 
`connection` is `LauncherConnection` used to report back Spark Job status to 
User App.  So, if this connection is lost - the User App requesting/waiting for 
the job completion is no longer available.  So User app is not available 
shutdown flag would decide whether to kill the Spark job or not.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2017-02-07 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r99841742
  
--- Diff: 
launcher/src/main/java/org/apache/spark/launcher/AbstractSparkAppHandle.java ---
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.launcher;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+public abstract class AbstractSparkAppHandle implements SparkAppHandle {
+  private static final Logger LOG = 
Logger.getLogger(AbstractSparkAppHandle.class.getName());
--- End diff --

`java.util.logging.Logger#getLogger` would complain if argument is not a 
String.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2016-11-09 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin @tgravescs, I have worked on the review comments. Please revisit 
the work and let me know if we need any further changes to this patch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15627: [SPARK-18099][YARN] Fail if same files added to distribu...

2016-11-08 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15627
  
@ueshin Sorry about this. The patch is available in 
https://github.com/apache/spark/pull/15810. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #12203: [SPARK-14423][YARN] Avoid same name files added to distr...

2016-11-08 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/12203
  
@vanzin, @jerryshao 
Sorry for breaking this functionality. I have the patch available with more 
unit tests added to ensure positive test case ensuring submission continues if 
unique files/archives are mentioned.

https://github.com/apache/spark/pull/15810



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15810: [SPARK-18357] Fix yarn files/archive broken issue...

2016-11-08 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request:

https://github.com/apache/spark/pull/15810

[SPARK-18357] Fix yarn files/archive broken issue andd unit tests

## What changes were proposed in this pull request?

The #15627 broke functionality with yarn --files --archives does not accept 
any files.
This patch ensures that --files and --archives accept unique files.

## How was this patch tested?

A. I added unit tests.
B. Also, manually tested --files with --archives to throw exception if 
duplicate files are specified and continue if unique files are specified.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kishorvpatil/spark SPARK18357

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/15810.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #15810


commit ddc6b9bbf061ca3e7fac710333aa5ecfa2c24870
Author: Kishor Patil <kpa...@yahoo-inc.com>
Date:   2016-11-08T17:11:12Z

Fix yarn files/archive broken issue andd unit tests




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2016-11-07 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r86870416
  
--- Diff: 
launcher/src/main/java/org/apache/spark/launcher/ChildThreadAppHandle.java ---
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.launcher;
+
+import java.util.logging.Logger;
+
+public class ChildThreadAppHandle extends AbstractSparkAppHandle {
+  private static final Logger LOG = 
Logger.getLogger(ChildThreadAppHandle.class.getName());
+  private Thread childThread;
+
+  public ChildThreadAppHandle(String secret, LauncherServer server) {
+super(server, secret);
+  }
+
+  @Override
+  public synchronized void kill() {
+if (!disposed) {
+  disconnect();
+}
+if (childThread != null) {
+  try {
+childThread.join(3000);
--- End diff --

`disconnect()` does not, but if shopIfShutdown is setup, we would like to 
give sufficient time for `childTread` to run shutdownHook as it detects the 
launcherserver is disconnected.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15627: [SPARK-18099][YARN] Fail if same files added to d...

2016-10-25 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request:

https://github.com/apache/spark/pull/15627

[SPARK-18099][YARN] Fail if same files added to distributed cache for 
--files and --archives

## What changes were proposed in this pull request?

During spark-submit, if yarn dist cache is instructed to add same file 
under --files and --archives, This code change ensures the spark yarn 
distributed cache behaviour is retained i.e. to warn and fail if same files is 
mentioned in both --files and --archives.

## How was this patch tested?

Manually tested: 
1. if same jar is mentioned in --jars and --files it will continue to 
submit the job. 
  - basically functionality [SPARK-14423] #12203 is unchanged 
2. if same file is mentioned in --files and --archives it will fail to 
submit the job.

Please review 
https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark before 
opening a pull request.

… under archives and files

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kishorvpatil/spark spark18099

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/15627.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #15627


commit 9bb16236ad7bbb982e0ffaa73899ebc11df9e6ee
Author: Kishor Patil <kpa...@yahoo-inc.com>
Date:   2016-10-25T18:19:46Z

Dist cache yarn during submit should throw error for adding same file under 
archives and files




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2016-10-19 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@tgravescs @vanzin , Thanks for all the comments.
I added `SparkApp` trait, more documentation and minor code fixes based on 
our suggestion.
Please review and let me know if I need any other changes. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2016-10-19 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r84178923
  
--- Diff: 
launcher/src/main/java/org/apache/spark/launcher/SparkSubmitRunner.java ---
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.launcher;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+
+class SparkSubmitRunner implements Runnable {
+  private Method main;
+  private final List args;
+
+  SparkSubmitRunner(Method main, List args) {
+this.main = main;
+this.args = args;
+  }
+  /**
+   *  Trying to see if method is available in the classpath.
+   */
+  protected static Method getSparkSubmitMain() throws 
ClassNotFoundException, NoSuchMethodException {
+  Class cls = Class.forName("org.apache.spark.deploy.SparkSubmit");
+  return cls.getDeclaredMethod("main", String[].class);
+  }
+
+  @Override
+  public void run() {
+try {
+  if(main == null) {
+main = getSparkSubmitMain();
+  }
+  Object argsObj = args.toArray(new String[args.size()]);
+  main.invoke(null, argsObj);
+} catch (IllegalAccessException illAcEx) {
--- End diff --

This is to simply enable early fail is `SparkSubmit` is not in classpath, 
instead of launched `SparkSubmitRunner` thread finding the classpath issue.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...

2016-10-17 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r83744591
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -760,7 +787,7 @@ private[spark] class Client(
   .foreach { case (k, v) => 
YarnSparkHadoopUtil.addPathToEnvironment(env, k, v) }
 
 // Keep this for backwards compatibility but users should move to the 
config
-sys.env.get("SPARK_YARN_USER_ENV").foreach { userEnvs =>
+sysEnvironment.get("SPARK_YARN_USER_ENV").foreach { userEnvs =>
--- End diff --

created https://issues.apache.org/jira/browse/SPARK-17979 - Remove 
deprecated support for config 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15009: [SPARK-17443][SPARK-11035] Stop Spark Application if lau...

2016-10-03 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15009
  
@vanzin @tgravescs, Sorry for the delay. I have made changes to isolate 
System properties while launching multiple applications in thread mode.  Also, 
tested this by running multiple apps one after another to test if the validate 
if System properties is shared.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #15069: [SPARK-17511] Yarn Dynamic Allocation: Avoid marking rel...

2016-09-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/15069
  
@tgravescs fixed the test name


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15069: [SPARK-17511] Yarn Dynamic Allocation: Avoid mark...

2016-09-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15069#discussion_r78802398
  
--- Diff: 
yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -506,36 +505,41 @@ private[yarn] class YarnAllocator(
 allocatedContainerToHostMap.put(containerId, executorHostname)
   }
 
-  if (launchContainers) {
-launcherPool.execute(new Runnable {
-  override def run(): Unit = {
-try {
-  new ExecutorRunnable(
-Some(container),
-conf,
-sparkConf,
-driverUrl,
-executorId,
-executorHostname,
-executorMemory,
-executorCores,
-appAttemptId.getApplicationId.toString,
-securityMgr,
-localResources
-  ).run()
-  updateInternalState()
-} catch {
-  case NonFatal(e) =>
-logError(s"Failed to launch executor $executorId on 
container $containerId", e)
-// Assigned container should be released immediately to 
avoid unnecessary resource
-// occupation.
-amClient.releaseAssignedContainer(containerId)
+  if (numExecutorsRunning < targetNumExecutors) {
--- End diff --

@tgravescs , I have added unit test for the same.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15069: [SPARK-17511] Yarn Dynamic Allocation: Avoid mark...

2016-09-12 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request:

https://github.com/apache/spark/pull/15069

[SPARK-17511] Yarn Dynamic Allocation: Avoid marking released container as 
Failed

## What changes were proposed in this pull request?

Due to race conditions, the ` assert(numExecutorsRunning <= 
targetNumExecutors)` can fail causing `AssertionError`. So removed the 
assertion, instead moved the conditional check before launching new container:
```
java.lang.AssertionError: assertion failed
at scala.Predef$.assert(Predef.scala:156)
at 
org.apache.spark.deploy.yarn.YarnAllocator$$anonfun$runAllocatedContainers$1.org$apache$spark$deploy$yarn$YarnAllocator$$anonfun$$updateInternalState$1(YarnAllocator.scala:489)
at 
org.apache.spark.deploy.yarn.YarnAllocator$$anonfun$runAllocatedContainers$1$$anon$1.run(YarnAllocator.scala:519)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
```
## How was this patch tested?
This was manually tested using a large ForkAndJoin job with Dynamic 
Allocation enabled to validate the failing job succeeds, without any such 
exception.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kishorvpatil/spark SPARK-17511

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/15069.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #15069






---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443] Stop Spark Application if launcher ...

2016-09-08 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r78066827
  
--- Diff: 
core/src/main/scala/org/apache/spark/launcher/LauncherBackend.scala ---
@@ -29,26 +31,63 @@ import org.apache.spark.util.{ThreadUtils, Utils}
  *
  * See `LauncherServer` for an explanation of how launcher communication 
works.
  */
-private[spark] abstract class LauncherBackend {
+private[spark] abstract class LauncherBackend extends Logging {
 
   private var clientThread: Thread = _
   private var connection: BackendConnection = _
   private var lastState: SparkAppHandle.State = _
+  private var stopFlag: Boolean = false
   @volatile private var _isConnected = false
 
   def connect(): Unit = {
 val port = sys.env.get(LauncherProtocol.ENV_LAUNCHER_PORT).map(_.toInt)
 val secret = sys.env.get(LauncherProtocol.ENV_LAUNCHER_SECRET)
+val stopFlag = 
sys.env.get(LauncherProtocol.ENV_LAUNCHER_STOP_FLAG).map(_.toBoolean)
 if (port != None && secret != None) {
-  val s = new Socket(InetAddress.getLoopbackAddress(), port.get)
+  if(stopFlag != None) {
+connect(port.get, secret.get, stopFlag.get)
+  } else {
+connect(port.get, secret.get)
+  }
+}
+  }
+
+  def connect(port: Int, secret: String): Unit = {
+if (port != None && secret != None) {
+  val s = new Socket(InetAddress.getLoopbackAddress(), port)
   connection = new BackendConnection(s)
-  connection.send(new Hello(secret.get, SPARK_VERSION))
+  connection.send(new Hello(secret, SPARK_VERSION))
   clientThread = LauncherBackend.threadFactory.newThread(connection)
   clientThread.start()
   _isConnected = true
+  if(stopFlag) {
+val shutdownHook: Runnable = new Runnable() {
+  def run {
+logInfo("LauncherBackend shutdown hook invoked..")
+try {
+  if(_isConnected && stopFlag) {
+onStopRequest()
+  }
+}
+catch {
+  case anotherIOE: IOException => {
+logInfo("Error while running LauncherBackend 
shutdownHook...", anotherIOE)
--- End diff --

fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443] Stop Spark Application if launcher ...

2016-09-08 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/15009#discussion_r78050727
  
--- Diff: 
core/src/main/scala/org/apache/spark/launcher/LauncherBackend.scala ---
@@ -71,6 +110,9 @@ private[spark] abstract class LauncherBackend {
 if (connection != null && lastState != state) {
   connection.send(new SetState(state))
   lastState = state
+  if(!_isConnected && stopFlag) {
--- End diff --

This code is invoked, when childProcess is launched with `LauncherServer` 
going down. test case scenario 1 - `startApplication` with request to 
`stopIfInterrupted`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #15009: [SPARK-17443] Stop Spark Application if launcher ...

2016-09-07 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request:

https://github.com/apache/spark/pull/15009

[SPARK-17443] Stop Spark Application if launcher goes down and use 
reflection

## What changes were proposed in this pull request?

1. `SparkLauncher` provides `stopIfInterrupted` method to allow ensuring 
`SparkApplication` stops if tracking process dies. 
2.  `SparkLauncher` provides `startApplicationInProcess` method to start 
application using reflection in another Thread instead of launching 
childProcess and relying on `spark-submit` binary.

(Please fill in changes proposed in this fix)


## How was this patch tested?

Manual tests.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kishorvpatil/spark SPARK-17443

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/15009.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #15009


commit 8986aa9b1cde7e116874eb4b9bf5cf6142a244a4
Author: Kishor Patil <kpa...@yahoo-inc.com>
Date:   2016-09-08T03:25:51Z

Stop Spark Application if launcher goes down and use reflection




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-07-21 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@nblintao , I am not sure why we had the check of existence of logs - 
specially when the reference to logs is static link for `stdout` and `stderr`.
Unless there is scenario where there could be multiple log entries and 
dynamic list of logs- i suggest that would require coding of different solution 
- mere check for existence and dropping out column altogether would not help. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-07-20 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@ajbozarth Thank you for review. I removed unused methods from the the 
javascript. 

I agree converting pages to `DataTables` is tricky and listing some 
standard way might be a huge help.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-07-18 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@tgravescs, I removed unwanted code/comment from the javascript.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-07-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@tgravescs , moved `formatBytes` to `utils.js`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-07-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@ajbozarth , I fixed the code for standalone mode as well. Sorry for not 
being fully aware of possible UI invocations. I will keep this in mind for 
future patches.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-07-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@tgravescs, fixed documentation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-07-13 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@tgravescs, I think addressed the comments mentioned above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13670: [SPARK-15951] Change Executors Page to use datata...

2016-07-13 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/13670#discussion_r70686785
  
--- Diff: docs/monitoring.md ---
@@ -288,7 +288,11 @@ where `[base-app-id]` is the YARN application ID.
   
   
 /applications/[app-id]/executors
-A list of all executors for the given application.
+A list of all active executors for the given application.
+  
+  
+/applications/[app-id]/executors
--- End diff --

fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13670: [SPARK-15951] Change Executors Page to use datata...

2016-07-12 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/13670#discussion_r70560042
  
--- Diff: 
core/src/main/resources/org/apache/spark/ui/static/executorspage-template.html 
---
@@ -0,0 +1,102 @@
+
+
+

[GitHub] spark pull request #13670: [SPARK-15951] Change Executors Page to use datata...

2016-07-12 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request:

https://github.com/apache/spark/pull/13670#discussion_r70560022
  
--- Diff: 
core/src/main/resources/org/apache/spark/ui/static/executorspage.js ---
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// this function works exactly the same as UIUtils.formatDuration
+function formatDuration(milliseconds, type) {
+  if(type !== 'display') return milliseconds;
+  if (milliseconds < 100) {
+return milliseconds + " ms";
+  }
+  var seconds = milliseconds * 1.0 / 1000;
+  if (seconds < 1) {
+return seconds.toFixed(1) + " s";
+  }
+  if (seconds < 60) {
+return seconds.toFixed(0) + " s";
+  }
+  var minutes = seconds / 60;
+  if (minutes < 10) {
+return minutes.toFixed(1) + " min";
+  } else if (minutes < 60) {
+return minutes.toFixed(0) + " min";
+  }
+  var hours = minutes / 60;
+  return hours.toFixed(1) + " h";
+}
+
+function formatStatus(status, type) {
+if(type !== 'display') return status;
+if(status) {
+return "Active"
+} else {
+return "Dead"
+}
+}
+
+function formatBytes(bytes,type) {
+if(type !== 'display') return bytes;
+if(bytes == 0) return '0 B';
+var k = 1000;
+var dm = 3;
+var sizes = ['B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'];
+var i = Math.floor(Math.log(bytes) / Math.log(k));
+return parseFloat((bytes / Math.pow(k, i)).toFixed(dm)) + ' ' + 
sizes[i];
+}
+
+
+jQuery.extend( jQuery.fn.dataTableExt.oSort, {
+"title-numeric-pre": function ( a ) {
+var x = a.match(/title="*(-?[0-9\.]+)/)[1];
+return parseFloat( x );
+},
+
+"title-numeric-asc": function ( a, b ) {
+return ((a < b) ? -1 : ((a > b) ? 1 : 0));
+},
+
+"title-numeric-desc": function ( a, b ) {
+return ((a < b) ? 1 : ((a > b) ? -1 : 0));
+}
+} );
+
+$(document).ajaxStop($.unblockUI);
+$(document).ajaxStart(function(){
+$.blockUI({ message: 'Loading Executors Page...'});
+});
+
+function createTemplateURI() {
+var parser = document.createElement('a');
+var words = parser.baseURI.split('/');
+var ind = words.indexOf("proxy");
+if(ind > 0) {
+var appId = words[ind + 1];
+var baseURI = words.slice(0, ind + 1).join('/') + '/' + appId + 
'/static/executorspage-template.html';
+return baseURI;
+} else {
+ind = words.indexOf("history");
+var baseURI = words.slice(0, ind).join('/') + 
'/static/executorspage-template.html';
+return baseURI;
+}
+}
+
+function createRESTEndPoint() {
+var parser = document.createElement('a');
+var words = parser.baseURI.split('/');
--- End diff --

As mentioned, I switched it to use `document.baseURI` which works with 
Safari as well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-07-12 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@ajbozarth, I have fixed the issue with safari exception. Its because 
`parser.baseURI` is `null`. I switched to `document.baseURI`. It looks fine on 
testing. Let me know if you still see this issue.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-06-28 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@ajbozarth, I do not see how I could produce the exception you see. Please 
help me reproduce this so that I can address it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-06-28 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@zhuoliu , i have removed unwanted, unused javascript functions in the 
executorspage.js. There are no duplicate functions now between historyserver.js 
and this script..


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #13670: [SPARK-15951] Change Executors Page to use datatables to...

2016-06-15 Thread kishorvpatil
Github user kishorvpatil commented on the issue:

https://github.com/apache/spark/pull/13670
  
@ajbozarth , Thank you for loooking into this patch. I have added 
screenshot with annotation. I hope that helps.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13670: [SPARK-15951] Change Executors Page to use datata...

2016-06-14 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request:

https://github.com/apache/spark/pull/13670

[SPARK-15951] Change Executors Page to use datatables to support sorting 
columns and searching

1. Create the executorspage-template.html for displaying application 
information in datables.
2. Added REST API endpoint "allexecutors" to be able to see all executors 
created for particular job.
3. The executorspage.js uses jQuery to access the data from 
/api/v1/applications/appid/allexecutors REST API, and use DataTable to display 
executors for the application. It also, generates summary of dead/live and 
total executors created during life of the application.
4. Similar changes applicable to Executors Page on history server for a 
given application.

Snapshots for how it looks like now:
https://cloud.githubusercontent.com/assets/6090397/16060092/ad1de03a-324b-11e6-8469-9eaa3f2548b5.png;>


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kishorvpatil/spark execTemplates

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/13670.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #13670


commit 9bc16141318dff854a258dc132087bce3c4f1a4b
Author: Kishor Patil <kpa...@yahoo-inc.com>
Date:   2016-06-07T19:31:35Z

Add AllExecutors REST Endpoint

commit 4a4fc89b3655b5ff4e47bacc39ad9d1eaf08b107
Author: Kishor Patil <kpa...@yahoo-inc.com>
Date:   2016-06-08T02:26:48Z

Adding ExecutorsPage template

commit 56f3901bcebdd158ba607fca20e8ef92b2468eb5
Author: Kishor Patil <kpa...@yahoo-inc.com>
Date:   2016-06-14T15:04:34Z

History page for Executors use mustache template




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org