[SPARK-8372] History server shows incorrect information for application not started #6827

carsonwang · 2015-06-15T08:40:50Z

The history server may show an incorrect App ID for an incomplete application like .inprogress. This app info will never disappear even after the app is completed.

The cause of the issue is that a log path name is used as the app id when app id cannot be got during replay.

jerryshao · 2015-06-15T08:48:00Z

core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala

@@ -282,8 +282,14 @@ private[history] class FsHistoryProvider(conf: SparkConf, clock: Clock)
    val newAttempts = logs.flatMap { fileStatus =>
      try {
        val res = replay(fileStatus, bus)
-        logInfo(s"Application log ${res.logPath} loaded successfully.")
-        Some(res)
+        res.map { r =>


I think it would be better to change to pattern match style, using map is little weird.

res match { case Some(r) => logInfo("...") case None => logInfo("...") }

carsonwang · 2015-06-15T09:20:20Z

Thanks @jerryshao . I updated the code as suggested.

SparkQA · 2015-06-15T11:17:28Z

Test build #34928 has finished for PR 6827 at commit 90f5dde.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-06-15T11:23:25Z

Test build #34930 has finished for PR 6827 at commit 3e46b35.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

markhamstra · 2015-06-15T16:05:33Z

@jerryshao The res.map { r => version would be considered idiomatic Scala style, not "weird". In this case, there's not a lot of reason to prefer one style over the other, but often the o.map{ ... }.getOrElse{ ... } style is the preferred way of dealing with Options.

vanzin · 2015-06-15T16:09:52Z

core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala

@@ -160,7 +160,7 @@ private[history] class FsHistoryProvider(conf: SparkConf, clock: Clock)
          replayBus.addListener(appListener)
          val appInfo = replay(fs.getFileStatus(new Path(logDir, attempt.logPath)), replayBus)

-          ui.setAppName(s"${appInfo.name} ($appId)")
+          appInfo.map { app => ui.setAppName(s"${app.name} ($appId)") }


Since you're not using the return value, we generally use Option.foreach, which is slightly cheaper.

vanzin · 2015-06-15T16:15:46Z

If the PR summary you mention:

This app info will never disappear even after the app is completed.

But I don't see that problem being fixed anywhere. The app id you set when getAppUI is called is never overridden while the app is in the HistoryServer.scala UI cache. If you don't plan to fix that, I suggest removing that from the summary (the fix is being tracked in SPARK-8275, btw).

jerryshao · 2015-06-16T00:46:41Z

Hi @markhamstra , I'm pretty sure about this style o.map{ ... }.getOrElse{ ... }, but here we transfer nothing but only print some different logs, so here using map is a little weird from my perspective, just my suggestion :).

markhamstra · 2015-06-16T01:30:29Z

Yes, @jerryshao, the idiom is a little clearer when the contents of the Option are actually used to produce a result other than Unit, but it's also a little odd to use a different idiom just to handle the Unit result type. A lot of this is just style preference differences that often stem from how familiar and comfortable a developer is with other functional programming languages, libraries and idioms. And don't get me started on whether fold over an Option is a better approach than map getOrElse -- we've already had that discussion, and I lost :(.

Anyway, the upshot of this is that map with a Unit result looks weird to some of us, unnecessary pattern matching over Option looks ugly to others of us, and Option#fold is so offensive to some that I can't use it in Spark!

carsonwang · 2015-06-16T01:57:00Z

@vanzin SPARK-8275 is a different issue. What I wanted to fix in this PR is to avoid showing an App ID like application_1432793609805_009_1.inprogress. This is not a valid App ID. The cause of this is in replay method, it uses appListener.appId.getOrElse(logPath.getName()), to get the App ID. If the SparkListenerApplicationStart event is not logged in the event log, the log path is used as the App ID. So I fixed this issue by make sure we only return a FsApplicationAttemptInfo when there is a valid App ID.

carsonwang · 2015-06-16T02:09:36Z

Thanks for reviewing this, @vanzin . The code is updated.

vanzin · 2015-06-16T02:18:22Z

Ok, now I get what the patch is doing. LGTM with latest changes.

SparkQA · 2015-06-16T04:03:01Z

Test build #34967 has finished for PR 6827 at commit cdbb089.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

andrewor14 · 2015-06-17T19:04:55Z

core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala

-        logInfo(s"Application log ${res.logPath} loaded successfully.")
-        Some(res)
+        res match {
+          case Some(r) => logDebug(s"Application log ${r.logPath} loaded successfully.")


should this be info?

oh never mind, just saw @vanzin's comment

andrewor14 · 2015-06-17T20:41:21Z

Ok I'm going to merge this into master 1.4 after addressing the comments myself thanks.

…on not started The history server may show an incorrect App ID for an incomplete application like <App ID>.inprogress. This app info will never disappear even after the app is completed. ![incorrectappinfo](https://cloud.githubusercontent.com/assets/9278199/8156147/2a10fdbe-137d-11e5-9620-c5b61d93e3c1.png) The cause of the issue is that a log path name is used as the app id when app id cannot be got during replay. Author: Carson Wang <carson.wang@intel.com> Closes #6827 from carsonwang/SPARK-8372 and squashes the following commits: cdbb089 [Carson Wang] Fix code style 3e46b35 [Carson Wang] Update code style 90f5dde [Carson Wang] Add a unit test d8c9cd0 [Carson Wang] Replaying events only return information when app is started (cherry picked from commit 2837e06) Signed-off-by: Andrew Or <andrew@databricks.com>

…on not started The history server may show an incorrect App ID for an incomplete application like <App ID>.inprogress. This app info will never disappear even after the app is completed. ![incorrectappinfo](https://cloud.githubusercontent.com/assets/9278199/8156147/2a10fdbe-137d-11e5-9620-c5b61d93e3c1.png) The cause of the issue is that a log path name is used as the app id when app id cannot be got during replay. Author: Carson Wang <carson.wang@intel.com> Closes apache#6827 from carsonwang/SPARK-8372 and squashes the following commits: cdbb089 [Carson Wang] Fix code style 3e46b35 [Carson Wang] Update code style 90f5dde [Carson Wang] Add a unit test d8c9cd0 [Carson Wang] Replaying events only return information when app is started

vanzin · 2015-06-25T16:33:32Z

So, one thing that I noticed after I said "LGTM" is that this change breaks old logs (those generated by versions of Spark that do not record the app id). Those will never show up anymore (I think that should only be Spark 1.0?).

If we care about that use case, this version should probably be reverted. The proper fix could be something as simple as this:

appListener.appId.getOrElse(logPath.getName().stripSuffix(EventLoggingListener.IN_PROGRESS)),

andrewor14 · 2015-06-29T17:38:31Z

@vanzin can you elaborate? Is there a fix for this without reverting the patch?

vanzin · 2015-06-29T17:43:27Z

@andrewor14 It's not a particular line, it's the whole patch. The patch ignores any application whose logs do not contain an application ID. No logs generated by Spark 1.0 contain an app id, so they're all ignored after this patch.

andrewor14 · 2015-06-29T17:51:19Z

OK, I'm going to revert this patch since the app ID is fundamental to the fix here. @vanzin would you mind submitting an alternative fix?

…on not started The history server may show an incorrect App ID for an incomplete application like <App ID>.inprogress. This app info will never disappear even after the app is completed. ![incorrectappinfo](https://cloud.githubusercontent.com/assets/9278199/8156147/2a10fdbe-137d-11e5-9620-c5b61d93e3c1.png) The cause of the issue is that a log path name is used as the app id when app id cannot be got during replay. Author: Carson Wang <carson.wang@intel.com> Closes apache#6827 from carsonwang/SPARK-8372 and squashes the following commits: cdbb089 [Carson Wang] Fix code style 3e46b35 [Carson Wang] Update code style 90f5dde [Carson Wang] Add a unit test d8c9cd0 [Carson Wang] Replaying events only return information when app is started

carsonwang added 2 commits June 15, 2015 16:23

Replaying events only return information when app is started

d8c9cd0

Add a unit test

90f5dde

jerryshao reviewed Jun 15, 2015
View reviewed changes

Update code style

3e46b35

vanzin reviewed Jun 15, 2015
View reviewed changes

Fix code style

cdbb089

andrewor14 reviewed Jun 17, 2015
View reviewed changes

asfgit closed this in 2837e06 Jun 17, 2015

andrewor14 mentioned this pull request Jun 30, 2015

[SPARK-8372] Do not show applications that haven't recorded their app ID yet. #7097

Closed

carsonwang deleted the SPARK-8372 branch August 17, 2015 01:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-8372] History server shows incorrect information for application not started #6827

[SPARK-8372] History server shows incorrect information for application not started #6827

carsonwang commented Jun 15, 2015

jerryshao Jun 15, 2015

carsonwang commented Jun 15, 2015

SparkQA commented Jun 15, 2015

SparkQA commented Jun 15, 2015

markhamstra commented Jun 15, 2015

vanzin Jun 15, 2015

vanzin commented Jun 15, 2015

jerryshao commented Jun 16, 2015

markhamstra commented Jun 16, 2015

carsonwang commented Jun 16, 2015

carsonwang commented Jun 16, 2015

vanzin commented Jun 16, 2015

SparkQA commented Jun 16, 2015

andrewor14 Jun 17, 2015

andrewor14 Jun 17, 2015

andrewor14 commented Jun 17, 2015

vanzin commented Jun 25, 2015

andrewor14 commented Jun 29, 2015

vanzin commented Jun 29, 2015

andrewor14 commented Jun 29, 2015

[SPARK-8372] History server shows incorrect information for application not started #6827

[SPARK-8372] History server shows incorrect information for application not started #6827

Conversation

carsonwang commented Jun 15, 2015

jerryshao Jun 15, 2015

Choose a reason for hiding this comment

carsonwang commented Jun 15, 2015

SparkQA commented Jun 15, 2015

SparkQA commented Jun 15, 2015

markhamstra commented Jun 15, 2015

vanzin Jun 15, 2015

Choose a reason for hiding this comment

vanzin commented Jun 15, 2015

jerryshao commented Jun 16, 2015

markhamstra commented Jun 16, 2015

carsonwang commented Jun 16, 2015

carsonwang commented Jun 16, 2015

vanzin commented Jun 16, 2015

SparkQA commented Jun 16, 2015

andrewor14 Jun 17, 2015

Choose a reason for hiding this comment

andrewor14 Jun 17, 2015

Choose a reason for hiding this comment

andrewor14 commented Jun 17, 2015

vanzin commented Jun 25, 2015

andrewor14 commented Jun 29, 2015

vanzin commented Jun 29, 2015

andrewor14 commented Jun 29, 2015