You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by vanzin <gi...@git.apache.org> on 2015/04/09 02:44:51 UTC

[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

GitHub user vanzin opened a pull request:

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

    [SPARK-4705] Handle multiple app attempts event logs, history server.

    This change modifies the event logging listener to write the logs for different application
    attempts to different files. The attempt ID is set by the scheduler backend, so as long
    as the backend returns that ID to SparkContext, things should work. Currently, the
    YARN backend does that.
    
    The history server was also modified to model multiple attempts per application. Each
    attempt has its own UI and a separate row in the listing table, so that users can look at
    all the attempts separately. The UI "adapts" itself to avoid showing attempt-specific info
    when all the applications being shown have a single attempt.


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

    $ git pull https://github.com/vanzin/spark SPARK-4705

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

    https://github.com/apache/spark/pull/5432.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 #5432
    
----
commit 0eb7722da9e84cb8485e081b241d8260eceba971
Author: twinkle sachdeva <tw...@guavus.com>
Date:   2015-02-02T15:38:35Z

    SPARK-4705: Doing cherry-pick of fix into master

commit 4c1fc262ea12ff6e4f16fd5ef66db12a7635716d
Author: twinkle sachdeva <tw...@kite.ggn.in.guavus.com>
Date:   2015-02-25T04:45:43Z

    SPARK-4705 Incorporating the review comments regarding formatting, will do the rest of the changes after this

commit 6b2e521b2b0e0485e41489e1b3471c363d4b999b
Author: twinkle sachdeva <tw...@kite.ggn.in.guavus.com>
Date:   2015-02-25T09:20:33Z

    SPARK-4705 Incorporating the review comments regarding formatting, will do the rest of the changes after this

commit 318525ad711d121e7edb4110c8ff433d47d0f59a
Author: twinkle.sachdeva <tw...@guavus.com>
Date:   2015-03-01T15:02:50Z

    SPARK-4705: 1) moved from directory structure to single file, as per the master branch. 2) Added the attempt id inside the SparkListenerApplicationStart, to make the info available independent of directory structure. 3) Changes in History Server to render the UI as per the snaphot II

commit 5fd5c6f44a56d2826e134cd03edcf5711558ecc6
Author: Marcelo Vanzin <va...@cloudera.com>
Date:   2015-04-07T23:59:31Z

    Fix my broken rebase.

commit 3245aa254bb2c9f2152db1d3be206c0d8e1f851d
Author: Marcelo Vanzin <va...@cloudera.com>
Date:   2015-04-08T18:24:41Z

    Make app attempts part of the history server model.
    
    This change explicitly models app attempts in the history server.
    An app is now a collection of app attempts, instead of the logic
    to match apps to app attempts being implicit in the rendering code.
    
    This makes the rendering code a lot simpler, since it doesn't need
    to do any fancy processing of the app list to figure out what to show.

commit 88b1de8d9ebfd3cef78161c9cbc4d3a06639a933
Author: Marcelo Vanzin <va...@cloudera.com>
Date:   2015-04-08T21:45:29Z

    Add a test for apps with multiple attempts.

commit cbe8bba9b0ad62f51fa4a742ac2b2665f730ffd1
Author: Marcelo Vanzin <va...@cloudera.com>
Date:   2015-04-08T22:01:28Z

    Attempt ID in listener event should be an option.
    
    For backwards compatibility.

commit ce5ee5de4206d74491ecd13411ca2947a81ab4e1
Author: Marcelo Vanzin <va...@cloudera.com>
Date:   2015-04-08T23:57:21Z

    Misc UI, test, style fixes.

commit c3e0a828a975053144479d023d3b639492119ac6
Author: Marcelo Vanzin <va...@cloudera.com>
Date:   2015-04-09T00:15:47Z

    Move app name to app info, more UI fixes.

commit 657ec18b5affa0a3f77f51a3e3e05ddbbc21c85f
Author: Marcelo Vanzin <va...@cloudera.com>
Date:   2015-04-09T00:27:26Z

    Fix yarn history URL, app links.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91358660
  
    btw i prefer #2.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29472825
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/EventLoggingListener.scala ---
    @@ -261,11 +267,20 @@ private[spark] object EventLoggingListener extends Logging {
       def getLogPath(
           logBaseDir: URI,
           appId: String,
    +      appAttemptId: Option[String],
           compressionCodecName: Option[String] = None): String = {
         val sanitizedAppId = appId.replaceAll("[ :/]", "-").replaceAll("[.${}'\"]", "_").toLowerCase
    --- End diff --
    
    can we call `sanitize` on this too?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29210659
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
    @@ -232,76 +239,105 @@ private[history] class FsHistoryProvider(conf: SparkConf) extends ApplicationHis
                 e)
               None
           }
    -    }.toSeq.sortWith(compareAppInfo)
    -
    -    // When there are new logs, merge the new list with the existing one, maintaining
    -    // the expected ordering (descending end time). Maintaining the order is important
    -    // to avoid having to sort the list every time there is a request for the log list.
    -    if (newApps.nonEmpty) {
    -      val mergedApps = new mutable.LinkedHashMap[String, FsApplicationHistoryInfo]()
    -      def addIfAbsent(info: FsApplicationHistoryInfo): Unit = {
    -        if (!mergedApps.contains(info.id) ||
    -            mergedApps(info.id).logPath.endsWith(EventLoggingListener.IN_PROGRESS) &&
    -            !info.logPath.endsWith(EventLoggingListener.IN_PROGRESS)) {
    -          mergedApps += (info.id -> info)
    -        }
    -      }
    +    }
     
    -      val newIterator = newApps.iterator.buffered
    -      val oldIterator = applications.values.iterator.buffered
    -      while (newIterator.hasNext && oldIterator.hasNext) {
    -        if (compareAppInfo(newIterator.head, oldIterator.head)) {
    -          addIfAbsent(newIterator.next())
    -        } else {
    -          addIfAbsent(oldIterator.next())
    +    if (newAttempts.isEmpty) {
    +      return
    +    }
    +
    +    // Build a map containing all apps that contain new attempts. The app information in this map
    +    // contains both the new app attempt, and those that were already loaded in the existing apps
    +    // map. If an attempt has been updated, it replaces the old attempt in the list.
    +    val newAppMap = new mutable.HashMap[String, FsApplicationHistoryInfo]()
    +    newAttempts.foreach { attempt =>
    +      val appInfo = newAppMap.get(attempt.appId)
    +        .orElse(applications.get(attempt.appId))
    +        .map { app =>
    +          val attempts =
    +            app.attempts.filter(_.attemptId != attempt.attemptId).toList ++ List(attempt)
    +          new FsApplicationHistoryInfo(attempt.appId, attempt.name,
    +            attempts.sortWith(compareAttemptInfo))
             }
    +        .getOrElse(new FsApplicationHistoryInfo(attempt.appId, attempt.name, List(attempt)))
    +      newAppMap(attempt.appId) = appInfo
    +    }
    +
    +    // Merge the new app list with the existing one, maintaining the expected ordering (descending
    +    // end time). Maintaining the order is important to avoid having to sort the list every time
    +    // there is a request for the log list.
    +    val newApps = newAppMap.values.toSeq.sortWith(compareAppInfo)
    +    val mergedApps = new mutable.LinkedHashMap[String, FsApplicationHistoryInfo]()
    +    def addIfAbsent(info: FsApplicationHistoryInfo): Unit = {
    +      if (!mergedApps.contains(info.id)) {
    +        mergedApps += (info.id -> info)
           }
    -      newIterator.foreach(addIfAbsent)
    -      oldIterator.foreach(addIfAbsent)
    +    }
     
    -      applications = mergedApps
    +    val newIterator = newApps.iterator.buffered
    +    val oldIterator = applications.values.iterator.buffered
    +    while (newIterator.hasNext && oldIterator.hasNext) {
    +      if (newAppMap.contains(oldIterator.head.id)) {
    +        oldIterator.next()
    +      } else if (compareAppInfo(newIterator.head, oldIterator.head)) {
    +        addIfAbsent(newIterator.next())
    +      } else {
    +        addIfAbsent(oldIterator.next())
    +      }
         }
    +    newIterator.foreach(addIfAbsent)
    +    oldIterator.foreach(addIfAbsent)
    +
    +    applications = mergedApps
       }
     
       /**
        * Delete event logs from the log directory according to the clean policy defined by the user.
        */
    -  private def cleanLogs(): Unit = {
    +  private[history] def cleanLogs(): Unit = {
         try {
           val maxAge = conf.getTimeAsSeconds("spark.history.fs.cleaner.maxAge", "7d") * 1000
     
    -      val now = System.currentTimeMillis()
    +      val now = clock.getTimeMillis()
           val appsToRetain = new mutable.LinkedHashMap[String, FsApplicationHistoryInfo]()
     
    +      def shouldClean(attempt: FsApplicationAttemptInfo): Boolean = {
    +        now - attempt.lastUpdated > maxAge && attempt.completed
    +      }
    +
           // Scan all logs from the log directory.
           // Only completed applications older than the specified max age will be deleted.
    -      applications.values.foreach { info =>
    -        if (now - info.lastUpdated <= maxAge || !info.completed) {
    -          appsToRetain += (info.id -> info)
    -        } else {
    -          appsToClean += info
    +      applications.values.foreach { app =>
    +        val toClean = app.attempts.filter(shouldClean)
    +        attemptsToClean ++= toClean
    +
    +        if (toClean.isEmpty) {
    +          appsToRetain += (app.id -> app)
    +        } else if (toClean.size < app.attempts.size) {
    +          appsToRetain += (app.id ->
    +            new FsApplicationHistoryInfo(app.id, app.name,
    +              app.attempts.filter(!shouldClean(_)).toList))
    --- End diff --
    
    nit: instead of filtering twice, you can do
    ```
    val  (toClean, toKeep) = app.attempts.partition(shouldClean)
    ```


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95736528
  
      [Test build #30876 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30876/consoleFull) for   PR 5432 at commit [`2ad77e7`](https://github.com/apache/spark/commit/2ad77e7f93aec5cc0f9d1b0136352fe22bab5683).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91080150
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29905/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97241850
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31166/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91358326
  
    so i just grepped through the code and found stuff like this:
    
    yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala:        YarnSparkHadoopUtil.expandEnvironment(Environment.JAVA_HOME) + "/bin/java", "-server"
    yarn/src/main/scala/org/apache/spark/deploy/yarn/ExecutorRunnable.scala:      YarnSparkHadoopUtil.expandEnvironment(Environment.JAVA_HOME) + "/bin/java",
    
    i've never explicitly set JAVA_HOME in jenkins' slave user space before, but that's obviously why it's failing.  that's pretty bad code imo.
    
    solutions:
    * explicitly set JAVA_HOME in each slave's config (bad, as it ties that slave to whatever is on system java)
    * if JAVA_HOME isn't set, use whatever java is in the path (good)
    * explicitly define which java version to test against in the jenkins build's 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97998849
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29473401
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala ---
    @@ -41,4 +41,11 @@ private[spark] trait SchedulerBackend {
        */
       def applicationId(): String = appId
     
    +  /**
    +   * Get an application ID associated with the job.
    +   *
    --- End diff --
    
    might be worth a comment even though that is the case the developer doesn't need to guess.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r28919320
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala ---
    @@ -41,4 +41,11 @@ private[spark] trait SchedulerBackend {
        */
       def applicationId(): String = appId
     
    +  /**
    +   * Get an application ID associated with the job.
    +   *
    +   * @return An application attempt id
    +   */
    +  def applicationAttemptId(): String = ""
    --- End diff --
    
    Yeah, I went back and forth on this bug forgot to revisit the code. I agree that `Option` would be better here, let me see how big a change that would be.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91668829
  
      [Test build #30043 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30043/consoleFull) for   PR 5432 at commit [`07446c6`](https://github.com/apache/spark/commit/07446c688f94ccdb98645b2e75960ab745caa339).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91080026
  
      [Test build #29905 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29905/consoleFull) for   PR 5432 at commit [`657ec18`](https://github.com/apache/spark/commit/657ec18b5affa0a3f77f51a3e3e05ddbbc21c85f).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-98003450
  
    Latest changes LGTM based on my reviews. @squito feel free to merge 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29232245
  
    --- Diff: core/src/test/scala/org/apache/spark/deploy/history/FsHistoryProviderSuite.scala ---
    @@ -159,52 +167,52 @@ class FsHistoryProviderSuite extends FunSuite with BeforeAndAfter with Matchers
       }
     
       test("SPARK-3697: ignore directories that cannot be read.") {
    -    val logFile1 = newLogFile("new1", inProgress = false)
    +    val logFile1 = newLogFile("new1", None, inProgress = false)
         writeFile(logFile1, true, None,
    -      SparkListenerApplicationStart("app1-1", None, 1L, "test"),
    +      SparkListenerApplicationStart("app1-1", None, 1L, "test", None),
           SparkListenerApplicationEnd(2L)
           )
    -    val logFile2 = newLogFile("new2", inProgress = false)
    +    val logFile2 = newLogFile("new2", None, inProgress = false)
         writeFile(logFile2, true, None,
    -      SparkListenerApplicationStart("app1-2", None, 1L, "test"),
    +      SparkListenerApplicationStart("app1-2", None, 1L, "test", None),
           SparkListenerApplicationEnd(2L)
           )
         logFile2.setReadable(false, false)
     
         val provider = new FsHistoryProvider(createTestConf())
    -    provider.checkForLogs()
    -
    -    val list = provider.getListing().toSeq
    -    list should not be (null)
    -    list.size should be (1)
    +    updateAndCheck(provider) { list =>
    +      list.size should be (1)
    +    }
       }
     
       test("history file is renamed from inprogress to completed") {
         val provider = new FsHistoryProvider(createTestConf())
     
    -    val logFile1 = newLogFile("app1", inProgress = true)
    +    val logFile1 = newLogFile("app1", None, inProgress = true)
         writeFile(logFile1, true, None,
    -      SparkListenerApplicationStart("app1", Some("app1"), 1L, "test"),
    +      SparkListenerApplicationStart("app1", Some("app1"), 1L, "test", None),
           SparkListenerApplicationEnd(2L)
         )
    -    provider.checkForLogs()
    -    val appListBeforeRename = provider.getListing()
    -    appListBeforeRename.size should be (1)
    -    appListBeforeRename.head.logPath should endWith(EventLoggingListener.IN_PROGRESS)
    +    updateAndCheck(provider) { list =>
    +      list.size should be (1)
    +      list.head.attempts.head.asInstanceOf[FsApplicationAttemptInfo].logPath should
    +        endWith(EventLoggingListener.IN_PROGRESS)
    +    }
     
    -    logFile1.renameTo(newLogFile("app1", inProgress = false))
    -    provider.checkForLogs()
    -    val appListAfterRename = provider.getListing()
    -    appListAfterRename.size should be (1)
    -    appListAfterRename.head.logPath should not endWith(EventLoggingListener.IN_PROGRESS)
    +    logFile1.renameTo(newLogFile("app1", None, inProgress = false))
    +    updateAndCheck(provider) { list =>
    +      list.size should be (1)
    +      list.head.attempts.head.asInstanceOf[FsApplicationAttemptInfo].logPath should not
    +        endWith(EventLoggingListener.IN_PROGRESS)
    +    }
       }
     
       test("SPARK-5582: empty log directory") {
         val provider = new FsHistoryProvider(createTestConf())
     
    -    val logFile1 = newLogFile("app1", inProgress = true)
    +    val logFile1 = newLogFile("app1", None, inProgress = true)
         writeFile(logFile1, true, None,
    -      SparkListenerApplicationStart("app1", Some("app1"), 1L, "test"),
    +      SparkListenerApplicationStart("app1", Some("app1"), 1L, "test", None),
           SparkListenerApplicationEnd(2L))
    --- End diff --
    
    also not your change, but -- doesn't `inProgress = true` imply no `SparkListenerApplicationEnd` event?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91079896
  
    This PR is an updated version of #4845. I rebased the code on top of current master, added the suggestions I made on the original PR, fixed a bunch of style nits and other issues, and added a couple of tests. Here's a screenshot:
    
    <img src="http://i.imgur.com/EUGSRKQ.png">


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95699688
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30870/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97207858
  
    lgtm
    
    I'll wait a day for any other feedback.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97140990
  
      [Test build #31146 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31146/consoleFull) for   PR 5432 at commit [`bc885b7`](https://github.com/apache/spark/commit/bc885b7295ade58f1e0c3159d4e5a2cd7658ef3b).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29473311
  
    --- Diff: yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnClusterSchedulerBackend.scala ---
    @@ -47,4 +47,12 @@ private[spark] class YarnClusterSchedulerBackend(
           super.applicationId
         }
     
    +  override def applicationAttemptId(): Option[String] =
    +    // In YARN Cluster mode, spark.yarn.app.attemptid is expect to be set
    +    // before user application is launched.
    +    // So, if spark.yarn.app.id is not set, it is something wrong.
    --- End diff --
    
    is this supposed to be `spark.yarn.app.attemptId` instead of just the `app.id`? Maybe a simpler way to put this is "The attempt ID is always set for YARN cluster applications"


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91342936
  
    Funny. All YARN tests (not just in this PR) are failing with this:
    
        /bin/bash: /bin/java: No such file or directory
    
    Wonder what changed in the environment since they were working before? (And why is github's user name search so useless it cannot autocomplete Shane's user 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-98004124
  
      [Test build #31480 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31480/consoleFull) for   PR 5432 at commit [`7e289fa`](https://github.com/apache/spark/commit/7e289fa1297aa21f6c8764e4a237eb3a674675e0).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-98003922
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-92553839
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30209/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97241845
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-94528809
  
      [Test build #30599 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30599/consoleFull) for   PR 5432 at commit [`c14ec19`](https://github.com/apache/spark/commit/c14ec19c9e54a2ef1920a6d0f465d7913f5b8299).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91317729
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29949/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97732149
  
    @andrewor14 did you have any comments on this?  otherwise I am ready to merge


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r28919004
  
    --- Diff: core/src/main/scala/org/apache/spark/util/JsonProtocol.scala ---
    @@ -194,7 +194,8 @@ private[spark] object JsonProtocol {
         ("App Name" -> applicationStart.appName) ~
         ("App ID" -> applicationStart.appId.map(JString(_)).getOrElse(JNothing)) ~
         ("Timestamp" -> applicationStart.time) ~
    -    ("User" -> applicationStart.sparkUser)
    +    ("User" -> applicationStart.sparkUser) ~
    +    ("appAttemptId" -> applicationStart.appAttemptId.map(JString(_)).getOrElse(JNothing))
    --- End diff --
    
    You should add a test case here: 
    https://github.com/apache/spark/blob/fbe7106d75c6a1624d10793fba6759703bc5c6e6/core/src/test/scala/org/apache/spark/util/JsonProtocolSuite.scala#L275


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91360942
  
    oh, i just had a thought:  i installed a couple of different versions of java through jenkins, and right now the tests are set in the config to use 'Default', which is system level java.  i bet this is why JAVA_HOME isn't being set and why the tests are failing.  @JoshRosen is going to set JAVA_HOME for us to get the builds green and then we can look a little deeper in to the problem.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95677480
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30863/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29473444
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala ---
    @@ -41,4 +41,11 @@ private[spark] trait SchedulerBackend {
        */
       def applicationId(): String = appId
     
    +  /**
    +   * Get an application ID associated with the job.
    +   *
    --- End diff --
    
    actually, does it make sense for applications running in client mode to have an attempt ID?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97967184
  
    Sorry I will look at this right 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-94070649
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30496/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-94052890
  
    /cc @andrewor14 @harishreedharan it would be great to have some eyes on this, thanks!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97977263
  
    Great to see this fixed @vanzin. My comments are mostly minor and unfortunately I don't have the time to do a closer review. How much more work do you imagine fixing this additionally for standalone mode would be? If it's not that much we should also fix that for 1.4 in separate 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29470843
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
    @@ -138,31 +142,34 @@ private[history] class FsHistoryProvider(conf: SparkConf) extends ApplicationHis
     
       override def getListing(): Iterable[FsApplicationHistoryInfo] = applications.values
     
    -  override def getAppUI(appId: String): Option[SparkUI] = {
    +  override def getAppUI(appId: String, attemptId: Option[String]): Option[SparkUI] = {
         try {
    -      applications.get(appId).map { info =>
    -        val replayBus = new ReplayListenerBus()
    -        val ui = {
    -          val conf = this.conf.clone()
    -          val appSecManager = new SecurityManager(conf)
    -          SparkUI.createHistoryUI(conf, replayBus, appSecManager, appId,
    -            s"${HistoryServer.UI_PATH_PREFIX}/$appId")
    -          // Do not call ui.bind() to avoid creating a new server for each application
    -        }
    +      applications.get(appId).flatMap { appInfo =>
    +        val attempts = appInfo.attempts.filter(_.attemptId == attemptId)
    +        attempts.headOption.map { attempt =>
    --- End diff --
    
    minor: this can just be `find` I think


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-92553828
  
      [Test build #30209 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30209/consoleFull) for   PR 5432 at commit [`86de638`](https://github.com/apache/spark/commit/86de638c3579fb2d5ef07548003d80e835fb2c18).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91404992
  
    Jenkins, retest this please.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29473221
  
    --- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
    @@ -89,6 +89,10 @@ private[spark] class ApplicationMaster(
     
             // Propagate the application ID so that YarnClusterSchedulerBackend can pick it up.
             System.setProperty("spark.yarn.app.id", appAttemptId.getApplicationId().toString())
    +
    +        // Propagate the attempt if, so that in case of event logging,
    +        // different attempt's logs gets created in different directory
    +        System.setProperty("spark.yarn.app.attemptid", appAttemptId.getAttemptId().toString())
    --- End diff --
    
    should this be `attemptId` camel case?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97987390
  
      [Test build #31464 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31464/consoleFull) for   PR 5432 at commit [`7e289fa`](https://github.com/apache/spark/commit/7e289fa1297aa21f6c8764e4a237eb3a674675e0).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-92541143
  
      [Test build #30209 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30209/consoleFull) for   PR 5432 at commit [`86de638`](https://github.com/apache/spark/commit/86de638c3579fb2d5ef07548003d80e835fb2c18).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91317719
  
      [Test build #29949 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29949/consoleFull) for   PR 5432 at commit [`9092af5`](https://github.com/apache/spark/commit/9092af517c07504a275f7d6ae2187fe97f8c83cd).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91111181
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29917/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91420020
  
      [Test build #29996 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29996/consoleFull) for   PR 5432 at commit [`9092af5`](https://github.com/apache/spark/commit/9092af517c07504a275f7d6ae2187fe97f8c83cd).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91697580
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30043/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29232672
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/ApplicationHistoryProvider.scala ---
    @@ -41,9 +45,11 @@ private[history] abstract class ApplicationHistoryProvider {
        * Returns the Spark UI for a specific application.
        *
        * @param appId The application ID.
    +   * @param attemptId The application attempt ID for apps with multiple attempts (or an empty
    +   *                  string for apps with a single attempt).
        * @return The application's UI, or None if application is not found.
        */
    -  def getAppUI(appId: String): Option[SparkUI]
    +  def getAppUI(appId: String, attemptId: String): Option[SparkUI]
    --- End diff --
    
    any reason not to use an `Option[String]` for `attemptId` here (and everywhere else) 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29472759
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
    @@ -76,18 +80,23 @@ class HistoryServer(
             return
           }
     
    -      val appId = parts(1)
    +      val appKey =
    +        if (parts.length == 3) {
    +          s"${parts(1)}/${parts(2)}"
    --- End diff --
    
    oh I see. IIUC this corresponds to `getAttemptURI` below


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-98003903
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91111161
  
      [Test build #29917 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29917/consoleFull) for   PR 5432 at commit [`3a14503`](https://github.com/apache/spark/commit/3a145030c7a2f729ce9665fdd454a83b21750c16).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91405879
  
      [Test build #29996 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29996/consoleFull) for   PR 5432 at commit [`9092af5`](https://github.com/apache/spark/commit/9092af517c07504a275f7d6ae2187fe97f8c83cd).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97166766
  
      [Test build #31146 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31146/consoleFull) for   PR 5432 at commit [`bc885b7`](https://github.com/apache/spark/commit/bc885b7295ade58f1e0c3159d4e5a2cd7658ef3b).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95763770
  
      [Test build #30880 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30880/consoleFull) for   PR 5432 at commit [`1aa309d`](https://github.com/apache/spark/commit/1aa309d4a3b29fc106629f035ce24c604853bf1c).
     * This patch **passes all tests**.
     * This patch **does not merge cleanly**.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91103821
  
      [Test build #29917 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29917/consoleFull) for   PR 5432 at commit [`3a14503`](https://github.com/apache/spark/commit/3a145030c7a2f729ce9665fdd454a83b21750c16).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95717585
  
    just chatted w/ Marcelo about this offline for a bit, one issue here is how apps get displayed when there are some complete and some running attempts -- which page do they go on, are they all grouped together, and what is the sort order within attempts?  Marcelo is going to follow up on that.
    
    Other than that one issue, this is looking good, I'll take another pass after those 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-94052867
  
      [Test build #30496 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30496/consoleFull) for   PR 5432 at commit [`9092d39`](https://github.com/apache/spark/commit/9092d39bf660f096f1a74f76d42a9760b415f3a1).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29232845
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
    @@ -138,31 +142,34 @@ private[history] class FsHistoryProvider(conf: SparkConf) extends ApplicationHis
     
       override def getListing(): Iterable[FsApplicationHistoryInfo] = applications.values
     
    -  override def getAppUI(appId: String): Option[SparkUI] = {
    +  override def getAppUI(appId: String, attemptId: String): Option[SparkUI] = {
         try {
    -      applications.get(appId).map { info =>
    -        val replayBus = new ReplayListenerBus()
    -        val ui = {
    -          val conf = this.conf.clone()
    -          val appSecManager = new SecurityManager(conf)
    -          SparkUI.createHistoryUI(conf, replayBus, appSecManager, appId,
    -            s"${HistoryServer.UI_PATH_PREFIX}/$appId")
    -          // Do not call ui.bind() to avoid creating a new server for each application
    -        }
    +      applications.get(appId).flatMap { appInfo =>
    +        val attempts = appInfo.attempts.filter(_.attemptId == attemptId)
    --- End diff --
    
    the doc for `getAppUI` says to use an empty string for apps with a single attempt -- but that isn't exactly what is reflected here.  Some yarn apps will be successful on the first attempt, but with this implementation, you still need to pass in the actual attempt id.  One way or the other, the doc & this should be resolved.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-94070634
  
      [Test build #30496 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30496/consoleFull) for   PR 5432 at commit [`9092d39`](https://github.com/apache/spark/commit/9092d39bf660f096f1a74f76d42a9760b415f3a1).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r28918841
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala ---
    @@ -41,4 +41,11 @@ private[spark] trait SchedulerBackend {
        */
       def applicationId(): String = appId
     
    +  /**
    +   * Get an application ID associated with the job.
    +   *
    +   * @return An application attempt id
    +   */
    +  def applicationAttemptId(): String = ""
    --- End diff --
    
    I think it would be cleaner if you used an `Option[String]` with a default of `None`.  As I was reading the rest of the code I was wondering if every framework would be forced to provide some dummy attempt id, or if would be null, etc.  `Option` would make that more explicit.  Plus it seems you use an option later in the listener event for json in any case.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95699663
  
      [Test build #30870 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30870/consoleFull) for   PR 5432 at commit [`9d59d92`](https://github.com/apache/spark/commit/9d59d92deff81863c8de58f8909bd0cf929b60c3).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29472977
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala ---
    @@ -41,4 +41,11 @@ private[spark] trait SchedulerBackend {
        */
       def applicationId(): String = appId
     
    +  /**
    +   * Get an application ID associated with the job.
    +   *
    --- End diff --
    
    when is this defined vs None? Is it as simple as "if the cluster manager provides it then it's defined, otherwise none"?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95677477
  
      [Test build #30863 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30863/consoleFull) for   PR 5432 at commit [`d5a9c37`](https://github.com/apache/spark/commit/d5a9c37a00f3b0b5aa66c5f92c325fdf0ac05bf0).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97199630
  
      [Test build #31166 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31166/consoleFull) for   PR 5432 at commit [`f66dcc5`](https://github.com/apache/spark/commit/f66dcc5b0625910d522de184b25800ecdf456366).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-94554600
  
      [Test build #30599 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30599/consoleFull) for   PR 5432 at commit [`c14ec19`](https://github.com/apache/spark/commit/c14ec19c9e54a2ef1920a6d0f465d7913f5b8299).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
      * `public class MapConfigProvider extends ConfigProvider `
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97998850
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31464/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97987098
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97987186
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91363859
  
    Ah yes, scratch that.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29473044
  
    --- Diff: core/src/main/scala/org/apache/spark/util/JsonProtocol.scala ---
    @@ -194,7 +194,8 @@ private[spark] object JsonProtocol {
         ("App Name" -> applicationStart.appName) ~
         ("App ID" -> applicationStart.appId.map(JString(_)).getOrElse(JNothing)) ~
         ("Timestamp" -> applicationStart.time) ~
    -    ("User" -> applicationStart.sparkUser)
    +    ("User" -> applicationStart.sparkUser) ~
    +    ("App Attempt ID" -> applicationStart.appAttemptId.map(JString(_)).getOrElse(JNothing))
    --- End diff --
    
    super minor but I would move this right under `App ID` since they're logically related.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95677173
  
      [Test build #30863 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30863/consoleFull) for   PR 5432 at commit [`d5a9c37`](https://github.com/apache/spark/commit/d5a9c37a00f3b0b5aa66c5f92c325fdf0ac05bf0).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29231263
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryPage.scala ---
    @@ -34,18 +37,28 @@ private[history] class HistoryPage(parent: HistoryServer) extends WebUIPage("")
         val requestedIncomplete =
           Option(request.getParameter("showIncomplete")).getOrElse("false").toBoolean
     
    -    val allApps = parent.getApplicationList().filter(_.completed != requestedIncomplete)
    -    val actualFirst = if (requestedFirst < allApps.size) requestedFirst else 0
    -    val apps = allApps.slice(actualFirst, Math.min(actualFirst + pageSize, allApps.size))
    +    val allApps = parent.getApplicationList()
    +      .filter(_.attempts.head.completed != requestedIncomplete)
    +    val allAppsSize = allApps.size
    +
    +    val actualFirst = if (requestedFirst < allAppsSize) requestedFirst else 0
    +    val appsToShow = allApps.slice(actualFirst, Math.min(actualFirst + pageSize, allAppsSize))
    --- End diff --
    
    I realize you didn't add this, but the `Math.min` isn't necessary:
    
    ```
    scala> (1 to 10).slice(5,20)
    res0: scala.collection.immutable.IndexedSeq[Int] = Vector(6, 7, 8, 9, 10)
    ```


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97166782
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29231506
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryPage.scala ---
    @@ -125,30 +138,85 @@ private[history] class HistoryPage(parent: HistoryServer) extends WebUIPage("")
         "Spark User",
         "Last Updated")
     
    -  private def rangeIndices(range: Seq[Int], condition: Int => Boolean, showIncomplete: Boolean):
    -  Seq[Node] = {
    +  private val appWithAttemptHeader = Seq(
    +    "App ID",
    +    "App Name",
    +    "Attempt ID",
    +    "Started",
    +    "Completed",
    +    "Duration",
    +    "Spark User",
    +    "Last Updated")
    +
    +  private def rangeIndices(
    +      range: Seq[Int],
    +      condition: Int => Boolean,
    +      showIncomplete: Boolean): Seq[Node] = {
         range.filter(condition).map(nextPage =>
           <a href={makePageLink(nextPage, showIncomplete)}> {nextPage} </a>)
       }
     
    -  private def appRow(info: ApplicationHistoryInfo): Seq[Node] = {
    -    val uiAddress = HistoryServer.UI_PATH_PREFIX + s"/${info.id}"
    -    val startTime = UIUtils.formatDate(info.startTime)
    -    val endTime = if (info.endTime > 0) UIUtils.formatDate(info.endTime) else "-"
    +  private def attemptRow(
    +      renderAttemptIdColumn: Boolean,
    +      info: ApplicationHistoryInfo,
    +      attempt: ApplicationAttemptInfo,
    +      isFirst: Boolean): Seq[Node] = {
    +    val uiAddress = HistoryServer.getAttemptURI(info.id, attempt.attemptId)
    +    val startTime = UIUtils.formatDate(attempt.startTime)
    +    val endTime = if (attempt.endTime > 0) UIUtils.formatDate(attempt.endTime) else "-"
         val duration =
    -      if (info.endTime > 0) UIUtils.formatDuration(info.endTime - info.startTime) else "-"
    -    val lastUpdated = UIUtils.formatDate(info.lastUpdated)
    +      if (attempt.endTime > 0) {
    +        UIUtils.formatDuration(attempt.endTime - attempt.startTime)
    +        } else {
    +          "-"
    +        }
    --- End diff --
    
    nit: fix indentation


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29473085
  
    --- Diff: core/src/test/scala/org/apache/spark/deploy/history/FsHistoryProviderSuite.scala ---
    @@ -96,33 +99,39 @@ class FsHistoryProviderSuite extends FunSuite with BeforeAndAfter with Matchers
         oldAppIncomplete.mkdir()
         createEmptyFile(new File(oldAppIncomplete, provider.SPARK_VERSION_PREFIX + "1.0"))
         writeFile(new File(oldAppIncomplete, provider.LOG_PREFIX + "1"), false, None,
    -      SparkListenerApplicationStart("old-app-incomplete", None, 2L, "test")
    +      SparkListenerApplicationStart("old-app-incomplete", None, 2L, "test", None)
           )
     
         // Force a reload of data from the log directory, and check that both logs are loaded.
         // Take the opportunity to check that the offset checks work as expected.
    -    provider.checkForLogs()
    +    updateAndCheck(provider) { list =>
    +      list.size should be (5)
    +      list.count(_.attempts.head.completed) should be (3)
    +
    +      def makeAppInfo(id: String, name: String, start: Long, end: Long, lastMod: Long,
    +        user: String, completed: Boolean): ApplicationHistoryInfo = {
    --- End diff --
    
    can you format this properly


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95763774
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30880/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91297856
  
      [Test build #29949 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29949/consoleFull) for   PR 5432 at commit [`9092af5`](https://github.com/apache/spark/commit/9092af517c07504a275f7d6ae2187fe97f8c83cd).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29472585
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryPage.scala ---
    @@ -22,6 +22,9 @@ import javax.servlet.http.HttpServletRequest
     import scala.xml.Node
     
     import org.apache.spark.ui.{WebUIPage, UIUtils}
    +import scala.collection.immutable.ListMap
    +import scala.collection.mutable.HashMap
    +import scala.collection.mutable.ArrayBuffer
    --- End diff --
    
    import order


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91360301
  
    cool.  on our systems, at least, the system java we use is /usr/bin/java, which points (through /etc/alternatives), to /usr/java/latest (which itself is a link to /usr/java/jdk1.7.0_71/).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91362958
  
    Note that the YARN code is not resolving `JAVA_HOME` locally, it's adding a reference to `$JAVA_HOME` to the command that will be executed by YARN. That will be resolved on the node where the command is run. The NM generally sets `JAVA_HOME` for child processes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91420024
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29996/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97241825
  
      [Test build #31166 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31166/consoleFull) for   PR 5432 at commit [`f66dcc5`](https://github.com/apache/spark/commit/f66dcc5b0625910d522de184b25800ecdf456366).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-94554611
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30599/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91080591
  
      [Test build #29907 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29907/consoleFull) for   PR 5432 at commit [`3a14503`](https://github.com/apache/spark/commit/3a145030c7a2f729ce9665fdd454a83b21750c16).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91697550
  
      [Test build #30043 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30043/consoleFull) for   PR 5432 at commit [`07446c6`](https://github.com/apache/spark/commit/07446c688f94ccdb98645b2e75960ab745caa339).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91081249
  
    BTW the zebra-striping in the UI looks a little broken right now, I'll take a look at that.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97998841
  
      [Test build #31464 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31464/consoleFull) for   PR 5432 at commit [`7e289fa`](https://github.com/apache/spark/commit/7e289fa1297aa21f6c8764e4a237eb3a674675e0).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95735979
  
      [Test build #30880 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30880/consoleFull) for   PR 5432 at commit [`1aa309d`](https://github.com/apache/spark/commit/1aa309d4a3b29fc106629f035ce24c604853bf1c).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95719510
  
      [Test build #30876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30876/consoleFull) for   PR 5432 at commit [`2ad77e7`](https://github.com/apache/spark/commit/2ad77e7f93aec5cc0f9d1b0136352fe22bab5683).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-96018033
  
    Hmmm... I'll have to fix the log cleaner. Shouldn't be terribly hard, and I'll also add tests for it on top, but feel free to look at the rest of the code in the meantime.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97979250
  
    > How much more work do you imagine fixing this additionally for standalone mode would be?
    
    I have no idea, I'm mostly unfamiliar with standalone cluster mode. Feel free to file a separate bug for 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95736540
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30876/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-95682582
  
      [Test build #30870 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30870/consoleFull) for   PR 5432 at commit [`9d59d92`](https://github.com/apache/spark/commit/9d59d92deff81863c8de58f8909bd0cf929b60c3).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29262574
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
    @@ -232,76 +239,105 @@ private[history] class FsHistoryProvider(conf: SparkConf) extends ApplicationHis
                 e)
               None
           }
    -    }.toSeq.sortWith(compareAppInfo)
    -
    -    // When there are new logs, merge the new list with the existing one, maintaining
    -    // the expected ordering (descending end time). Maintaining the order is important
    -    // to avoid having to sort the list every time there is a request for the log list.
    -    if (newApps.nonEmpty) {
    -      val mergedApps = new mutable.LinkedHashMap[String, FsApplicationHistoryInfo]()
    -      def addIfAbsent(info: FsApplicationHistoryInfo): Unit = {
    -        if (!mergedApps.contains(info.id) ||
    -            mergedApps(info.id).logPath.endsWith(EventLoggingListener.IN_PROGRESS) &&
    -            !info.logPath.endsWith(EventLoggingListener.IN_PROGRESS)) {
    -          mergedApps += (info.id -> info)
    -        }
    -      }
    +    }
     
    -      val newIterator = newApps.iterator.buffered
    -      val oldIterator = applications.values.iterator.buffered
    -      while (newIterator.hasNext && oldIterator.hasNext) {
    -        if (compareAppInfo(newIterator.head, oldIterator.head)) {
    -          addIfAbsent(newIterator.next())
    -        } else {
    -          addIfAbsent(oldIterator.next())
    +    if (newAttempts.isEmpty) {
    +      return
    +    }
    +
    +    // Build a map containing all apps that contain new attempts. The app information in this map
    +    // contains both the new app attempt, and those that were already loaded in the existing apps
    +    // map. If an attempt has been updated, it replaces the old attempt in the list.
    +    val newAppMap = new mutable.HashMap[String, FsApplicationHistoryInfo]()
    +    newAttempts.foreach { attempt =>
    +      val appInfo = newAppMap.get(attempt.appId)
    +        .orElse(applications.get(attempt.appId))
    +        .map { app =>
    +          val attempts =
    +            app.attempts.filter(_.attemptId != attempt.attemptId).toList ++ List(attempt)
    +          new FsApplicationHistoryInfo(attempt.appId, attempt.name,
    +            attempts.sortWith(compareAttemptInfo))
             }
    +        .getOrElse(new FsApplicationHistoryInfo(attempt.appId, attempt.name, List(attempt)))
    +      newAppMap(attempt.appId) = appInfo
    +    }
    +
    +    // Merge the new app list with the existing one, maintaining the expected ordering (descending
    +    // end time). Maintaining the order is important to avoid having to sort the list every time
    +    // there is a request for the log list.
    +    val newApps = newAppMap.values.toSeq.sortWith(compareAppInfo)
    +    val mergedApps = new mutable.LinkedHashMap[String, FsApplicationHistoryInfo]()
    +    def addIfAbsent(info: FsApplicationHistoryInfo): Unit = {
    +      if (!mergedApps.contains(info.id)) {
    +        mergedApps += (info.id -> info)
           }
    -      newIterator.foreach(addIfAbsent)
    -      oldIterator.foreach(addIfAbsent)
    +    }
     
    -      applications = mergedApps
    +    val newIterator = newApps.iterator.buffered
    +    val oldIterator = applications.values.iterator.buffered
    +    while (newIterator.hasNext && oldIterator.hasNext) {
    +      if (newAppMap.contains(oldIterator.head.id)) {
    +        oldIterator.next()
    +      } else if (compareAppInfo(newIterator.head, oldIterator.head)) {
    +        addIfAbsent(newIterator.next())
    +      } else {
    +        addIfAbsent(oldIterator.next())
    +      }
         }
    +    newIterator.foreach(addIfAbsent)
    +    oldIterator.foreach(addIfAbsent)
    +
    +    applications = mergedApps
       }
     
       /**
        * Delete event logs from the log directory according to the clean policy defined by the user.
        */
    -  private def cleanLogs(): Unit = {
    +  private[history] def cleanLogs(): Unit = {
         try {
           val maxAge = conf.getTimeAsSeconds("spark.history.fs.cleaner.maxAge", "7d") * 1000
     
    -      val now = System.currentTimeMillis()
    +      val now = clock.getTimeMillis()
           val appsToRetain = new mutable.LinkedHashMap[String, FsApplicationHistoryInfo]()
     
    +      def shouldClean(attempt: FsApplicationAttemptInfo): Boolean = {
    +        now - attempt.lastUpdated > maxAge && attempt.completed
    +      }
    +
           // Scan all logs from the log directory.
           // Only completed applications older than the specified max age will be deleted.
    -      applications.values.foreach { info =>
    -        if (now - info.lastUpdated <= maxAge || !info.completed) {
    -          appsToRetain += (info.id -> info)
    -        } else {
    -          appsToClean += info
    +      applications.values.foreach { app =>
    +        val toClean = app.attempts.filter(shouldClean)
    +        attemptsToClean ++= toClean
    +
    +        if (toClean.isEmpty) {
    +          appsToRetain += (app.id -> app)
    +        } else if (toClean.size < app.attempts.size) {
    +          appsToRetain += (app.id ->
    +            new FsApplicationHistoryInfo(app.id, app.name,
    +              app.attempts.filter(!shouldClean(_)).toList))
    --- End diff --
    
    I swear I read the whole API for `Seq` and missed that one...


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29472905
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/EventLoggingListener.scala ---
    @@ -261,11 +267,20 @@ private[spark] object EventLoggingListener extends Logging {
       def getLogPath(
           logBaseDir: URI,
           appId: String,
    +      appAttemptId: Option[String],
           compressionCodecName: Option[String] = None): String = {
         val sanitizedAppId = appId.replaceAll("[ :/]", "-").replaceAll("[.${}'\"]", "_").toLowerCase
    --- End diff --
    
    actually I don't think this variable is used


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97208646
  
    @vanzin thanks for the fix. I'll have a quick look at this tonight.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29264211
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
    @@ -138,31 +142,34 @@ private[history] class FsHistoryProvider(conf: SparkConf) extends ApplicationHis
     
       override def getListing(): Iterable[FsApplicationHistoryInfo] = applications.values
     
    -  override def getAppUI(appId: String): Option[SparkUI] = {
    +  override def getAppUI(appId: String, attemptId: String): Option[SparkUI] = {
         try {
    -      applications.get(appId).map { info =>
    -        val replayBus = new ReplayListenerBus()
    -        val ui = {
    -          val conf = this.conf.clone()
    -          val appSecManager = new SecurityManager(conf)
    -          SparkUI.createHistoryUI(conf, replayBus, appSecManager, appId,
    -            s"${HistoryServer.UI_PATH_PREFIX}/$appId")
    -          // Do not call ui.bind() to avoid creating a new server for each application
    -        }
    +      applications.get(appId).flatMap { appInfo =>
    +        val attempts = appInfo.attempts.filter(_.attemptId == attemptId)
    --- End diff --
    
    The interface doc is slightly misleading, but all event logs from YARN will have an attempt ID after this change, even for a single attempt.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29263018
  
    --- Diff: core/src/test/scala/org/apache/spark/deploy/history/FsHistoryProviderSuite.scala ---
    @@ -159,52 +167,52 @@ class FsHistoryProviderSuite extends FunSuite with BeforeAndAfter with Matchers
       }
     
       test("SPARK-3697: ignore directories that cannot be read.") {
    -    val logFile1 = newLogFile("new1", inProgress = false)
    +    val logFile1 = newLogFile("new1", None, inProgress = false)
         writeFile(logFile1, true, None,
    -      SparkListenerApplicationStart("app1-1", None, 1L, "test"),
    +      SparkListenerApplicationStart("app1-1", None, 1L, "test", None),
           SparkListenerApplicationEnd(2L)
           )
    -    val logFile2 = newLogFile("new2", inProgress = false)
    +    val logFile2 = newLogFile("new2", None, inProgress = false)
         writeFile(logFile2, true, None,
    -      SparkListenerApplicationStart("app1-2", None, 1L, "test"),
    +      SparkListenerApplicationStart("app1-2", None, 1L, "test", None),
           SparkListenerApplicationEnd(2L)
           )
         logFile2.setReadable(false, false)
     
         val provider = new FsHistoryProvider(createTestConf())
    -    provider.checkForLogs()
    -
    -    val list = provider.getListing().toSeq
    -    list should not be (null)
    -    list.size should be (1)
    +    updateAndCheck(provider) { list =>
    +      list.size should be (1)
    +    }
       }
     
       test("history file is renamed from inprogress to completed") {
         val provider = new FsHistoryProvider(createTestConf())
     
    -    val logFile1 = newLogFile("app1", inProgress = true)
    +    val logFile1 = newLogFile("app1", None, inProgress = true)
         writeFile(logFile1, true, None,
    -      SparkListenerApplicationStart("app1", Some("app1"), 1L, "test"),
    +      SparkListenerApplicationStart("app1", Some("app1"), 1L, "test", None),
           SparkListenerApplicationEnd(2L)
         )
    -    provider.checkForLogs()
    -    val appListBeforeRename = provider.getListing()
    -    appListBeforeRename.size should be (1)
    -    appListBeforeRename.head.logPath should endWith(EventLoggingListener.IN_PROGRESS)
    +    updateAndCheck(provider) { list =>
    +      list.size should be (1)
    +      list.head.attempts.head.asInstanceOf[FsApplicationAttemptInfo].logPath should
    +        endWith(EventLoggingListener.IN_PROGRESS)
    +    }
     
    -    logFile1.renameTo(newLogFile("app1", inProgress = false))
    -    provider.checkForLogs()
    -    val appListAfterRename = provider.getListing()
    -    appListAfterRename.size should be (1)
    -    appListAfterRename.head.logPath should not endWith(EventLoggingListener.IN_PROGRESS)
    +    logFile1.renameTo(newLogFile("app1", None, inProgress = false))
    +    updateAndCheck(provider) { list =>
    +      list.size should be (1)
    +      list.head.attempts.head.asInstanceOf[FsApplicationAttemptInfo].logPath should not
    +        endWith(EventLoggingListener.IN_PROGRESS)
    +    }
       }
     
       test("SPARK-5582: empty log directory") {
         val provider = new FsHistoryProvider(createTestConf())
     
    -    val logFile1 = newLogFile("app1", inProgress = true)
    +    val logFile1 = newLogFile("app1", None, inProgress = true)
         writeFile(logFile1, true, None,
    -      SparkListenerApplicationStart("app1", Some("app1"), 1L, "test"),
    +      SparkListenerApplicationStart("app1", Some("app1"), 1L, "test", None),
           SparkListenerApplicationEnd(2L))
    --- End diff --
    
    Not necessarily. `inProgress = true` just affects the log file 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97199348
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-98003267
  
    retest this please


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5432#discussion_r29472651
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
    @@ -76,18 +80,23 @@ class HistoryServer(
             return
           }
     
    -      val appId = parts(1)
    +      val appKey =
    +        if (parts.length == 3) {
    +          s"${parts(1)}/${parts(2)}"
    --- End diff --
    
    can you add a comment on what these parts represent? Maybe add an example in the comment?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97199328
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-97166787
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31146/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91359524
  
    I think `JAVA_HOME` is something that YARN exposes to all containers, so even if you don't set it for your application, that code should still work. I think we problem here is a little different - we should just make sure the tests have the same env as you'd find in an usual YARN installation.
    
    Anyway, I'm trying something out in #5441.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91080147
  
      [Test build #29905 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29905/consoleFull) for   PR 5432 at commit [`657ec18`](https://github.com/apache/spark/commit/657ec18b5affa0a3f77f51a3e3e05ddbbc21c85f).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class SparkListenerApplicationStart(appName: String, appId: Option[String],`
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4705] Handle multiple app attempts even...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5432#issuecomment-91361393
  
    Is it always safe to rely on `java.home` pointing to the right directory? IIUC this is independently of whether we use Maven or SBT. Then perhaps the correct way of fixing this is doing something like what `AbstractCommandBuilder` does, where if `JAVA_HOME` is not set it defaults to using `java.home`:https://github.com/apache/spark/blob/470d7453a56c56a41b2851551fe1830065f88b2c/launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java#L93


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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