[GitHub] spark pull request #15009: [SPARK-17443][SPARK-11035] Stop Spark Application...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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 ...
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 ...
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 ...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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