SPARK-2450 Adds executor log links to Web UI #3486

ksakellis · 2014-11-26T19:40:19Z

Adds links to stderr/stdout in the executor tab of the webUI for:

Standalone
Yarn client
Yarn cluster

This tries to add the log url support in a general way so as to make it easy to add support for all the
cluster managers. This is done by using environment variables to pass to the executor the log urls. The
SPARK_LOG_URL_ prefix is used and so additional logs besides stderr/stdout can also be added.

To propagate this information to the UI we use the onExecutorAdded spark listener event.

Although this commit doesn't add log urls when running on a mesos cluster, it should be possible to add using the same mechanism.

AmplabJenkins · 2014-11-26T19:42:11Z

Can one of the admins verify this patch?

ksakellis · 2014-11-26T22:19:18Z

andrewor14 · 2014-11-30T02:02:32Z

add to whitelist. Great to see this being implemented

SparkQA · 2014-11-30T02:10:24Z

Test build #23952 has started for PR 3486 at commit e78308e.

This patch merges cleanly.

SparkQA · 2014-11-30T03:33:01Z

Test build #23952 has finished for PR 3486 at commit e78308e.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerExecutorAdded(executorId: String, executorInfo : ExecutorInfo)
- case class SparkListenerExecutorRemoved(executorId: String, executorInfo : ExecutorInfo)
- case class RegisterExecutor(executorId: String, hostPort: String, cores: Int,
- class ExecutorInfo(
- class ExecutorsListener(storageStatusListener: StorageStatusListener) extends SparkListener

AmplabJenkins · 2014-11-30T03:33:05Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23952/
Test FAILed.

sryza · 2014-12-01T20:54:04Z

core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala

+  def extractLogUrls : Map[String, String] = {
+    val prefix = "SPARK_LOG_URL_"
+    sys.env.filterKeys(_.startsWith(prefix))
+           .map(e => (e._1.substring(prefix.length).toLowerCase, e._2))


should only be indented 2 spaces

sryza · 2014-12-01T21:15:30Z

SparkListener already has a onBlockManagerAdded callback. I wonder whether it makes sense to consolidate this with the onExecutorAdded callback you added. @pwendell any opinions?

SparkQA · 2014-12-02T00:57:53Z

Test build #24011 has started for PR 3486 at commit 864fc47.

This patch merges cleanly.

SparkQA · 2014-12-02T01:20:13Z

Test build #24012 has started for PR 3486 at commit 501f284.

This patch merges cleanly.

SparkQA · 2014-12-02T02:15:07Z

Test build #24011 has finished for PR 3486 at commit 864fc47.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerExecutorAdded(executorId: String, executorInfo : ExecutorInfo)
- case class SparkListenerExecutorRemoved(executorId: String, executorInfo : ExecutorInfo)
- case class RegisterExecutor(executorId: String, hostPort: String, cores: Int,
- class ExecutorInfo(
- class ExecutorsListener(storageStatusListener: StorageStatusListener) extends SparkListener

AmplabJenkins · 2014-12-02T02:15:11Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24011/
Test PASSed.

SparkQA · 2014-12-02T02:40:16Z

Test build #24012 has finished for PR 3486 at commit 501f284.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerExecutorAdded(executorId: String, executorInfo : ExecutorInfo)
- case class SparkListenerExecutorRemoved(executorId: String, executorInfo : ExecutorInfo)
- case class RegisterExecutor(executorId: String, hostPort: String, cores: Int,
- class ExecutorInfo(

AmplabJenkins · 2014-12-02T02:40:20Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24012/
Test PASSed.

vanzin · 2014-12-08T22:05:02Z

core/src/main/scala/org/apache/spark/scheduler/SparkListener.scala

+  /**
+   * Called when the driver registers a new executor.
+   */
+  def onExecutorAdded(executorAdded: SparkListenerExecutorAdded) { }


Hmmm. This is going to be one of those cases where it breaks existing code that extends this class. Not sure if there's a good workaround (even though it is marked as @DeveloperApi). :-/

BTW doesn't this break the build? There are a few listeners in Spark code itself (e.g. EventLoggingListener) which should have broken because of this.

(BTW fixing that listener means you'll probably need to touch JsonProtocol to serialize these new events to the event log... and you'll need to be careful not to keep the log URLs in the replayed UIs since they'll most probably be broken links at that point. Meaning that probably the UI listener should nuke the log URLs when the "executor removed" message is handled.)

Ah wait. I see. These methods have default implementations, so they'll only affect people extending SparkListener from Java. Still, we should probably save these events to the log for replay later.

andrewor14 · 2014-12-09T19:55:08Z

Hey @ksakellis is this still WIP? Should I start doing a detailed review yet?

ksakellis · 2014-12-09T21:00:57Z

@andrewor14 Yes you can start doing a review. I wanted to post this just to get sign off on the strategy i'm using. If people feel good about this, i'll post more unit tests/fix comments before asking for it to be merged.

andrewor14 · 2014-12-09T21:16:36Z

core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala

    context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
  }

+  def extractLogUrls : Map[String, String] = {


no space before :

SparkQA · 2015-02-04T23:47:09Z

Test build #26782 has finished for PR 3486 at commit df5ce42.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class RegisterExecutor(

AmplabJenkins · 2015-02-04T23:47:13Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26782/
Test FAILed.

SparkQA · 2015-02-04T23:47:50Z

Test build #26777 timed out for PR 3486 at commit 4d87c3b after a configured wait of 120m.

AmplabJenkins · 2015-02-04T23:47:53Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26777/
Test FAILed.

JoshRosen · 2015-02-05T07:13:10Z

Hi @ksakellis,

This patch looks like it's in pretty good shape overall.

When displaying the UI in the HistoryServer or the standalone Master's display of completed application UIs, the executor log links might not always work (e.g. if the worker serving the logs is dead). This will sometimes work, though, since it might be common for the standalone worker or Yarn node daemons to outlive the application. I don't see a great solution for this, but I'm open to any ideas / suggestions. I don't think this should necessarily block this PR, though.

I think I understand the rationale for why we use environment variables for passing the log viewer links: the CoarseGrainedSchedulerBackend, which posts the ExecutorAdded event, is used in both Yarn and Standalone modes and therefore can't know how to map executor ids to container log paths (since it doesn't know about node manager URLs). However, each deployment mode has a different backend for launching executors, so we can determine the URL there and pass it back to the driver. I agree with Andrew that this dataflow seems a bit non-intuitive, but I don't necessarily see a better way to handle this without modifying a bunch of other components. I don't think that the current design precludes us from changing this in the future. We are kind of locking ourselves into exposing the logName => logUrl map in a listener event, but this seems reasonable (this is independent of how we get the data backing this event to the source where the event is posted to the bus).

Two minor code changes that we need:

The private[ui] that I removed turns out to be necessary for MiMa checks to pass. Can you add this back in and add comment explaining why this is needed?
Could you revert 04e410e, my patch for the JsonProtocol compatibility, since it turns out that we don't need this because we only modify new-in-1.3 features?
If you don't mind, could you squash my changes into a single commit so that the merge script attributes authorship properly by default? I can also fix up the authorship attribution myself if I merge this PR.

Sorry for the mess due to my earlier changes.

Adds links to stderr/stdout in the executor tab of the webUI for: 1) Standalone 2) Yarn client 3) Yarn cluster This tries to add the log url support in a general way so as to make it easy to add support for all the cluster managers. This is done by using environment variables to pass to the executor the log urls. The SPARK_LOG_URL_ prefix is used and so additional logs besides stderr/stdout can also be added. To propagate this information to the UI we use the onExecutorAdded spark listener event. Although this commit doesn't add log urls when running on a mesos cluster, it should be possible to add using the same mechanism.

Reset listener after each test Don't null listener out at end of main().

SparkQA · 2015-02-06T02:07:38Z

Test build #26883 has started for PR 3486 at commit d190936.

This patch merges cleanly.

SparkQA · 2015-02-06T03:39:35Z

Test build #26883 has finished for PR 3486 at commit d190936.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-06T03:39:40Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26883/
Test FAILed.

JoshRosen · 2015-02-06T04:17:01Z

Jenkins, retest this please.

SparkQA · 2015-02-06T04:17:55Z

Test build #26894 has started for PR 3486 at commit d190936.

This patch merges cleanly.

SparkQA · 2015-02-06T05:30:40Z

Test build #26894 has finished for PR 3486 at commit d190936.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class RegisterExecutor(

AmplabJenkins · 2015-02-06T05:30:44Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26894/
Test PASSed.

JoshRosen · 2015-02-06T19:12:17Z

@ksakellis Thanks for cleaning up the mess from my earlier commits.

This looks good to me, so I'm going to merge it into master (1.4.0) and branch-1.3 (1.3.0). Thanks!

Adds links to stderr/stdout in the executor tab of the webUI for: 1) Standalone 2) Yarn client 3) Yarn cluster This tries to add the log url support in a general way so as to make it easy to add support for all the cluster managers. This is done by using environment variables to pass to the executor the log urls. The SPARK_LOG_URL_ prefix is used and so additional logs besides stderr/stdout can also be added. To propagate this information to the UI we use the onExecutorAdded spark listener event. Although this commit doesn't add log urls when running on a mesos cluster, it should be possible to add using the same mechanism. Author: Kostas Sakellis <[email protected]> Author: Josh Rosen <[email protected]> Closes #3486 from ksakellis/kostas-spark-2450 and squashes the following commits: d190936 [Josh Rosen] Fix a few minor style / formatting nits. Reset listener after each test Don't null listener out at end of main(). 8673fe1 [Kostas Sakellis] CR feedback. Hide the log column if there are no logs available 5bf6952 [Kostas Sakellis] [SPARK-2450] [CORE] Adds exeuctor log links to Web UI (cherry picked from commit 32e964c) Signed-off-by: Josh Rosen <[email protected]>

This was caused because #3486 added a new field to ExecutorInfo and #4369 added new tests that created ExecutorInfos. These patches were merged in quick succession and were never tested together, hence the compilation error.

JoshRosen · 2015-03-05T00:49:06Z

core/src/main/scala/org/apache/spark/deploy/worker/ExecutorRunner.scala

@@ -134,6 +135,12 @@ private[spark] class ExecutorRunner(
      // In case we are running this from within the Spark Shell, avoid creating a "scala"
      // parent process for the executor command
      builder.environment.put("SPARK_LAUNCH_WITH_SCALA", "0")
+
+      // Add webUI log urls
+      val baseUrl = s"http://$host:$webUiPort/logPage/?appId=$appId&executorId=$execId&logType="


It looks like $host here might be a machine's hostname, but we probably want this to reflect SPARK_PUBLIC_DNS (or whatever the new system property equivalent is) so that we generate an externally-accessible URL.

sryza reviewed Dec 1, 2014
View reviewed changes

ksakellis force-pushed the kostas-spark-2450 branch from e78308e to 864fc47 Compare December 2, 2014 00:55

vanzin reviewed Dec 8, 2014
View reviewed changes

andrewor14 reviewed Dec 9, 2014
View reviewed changes

ksakellis mentioned this pull request Dec 9, 2014

[SPARK-4765] Make GC time always shown in UI. #3622

Closed

Kostas Sakellis and others added 3 commits February 5, 2015 17:25

CR feedback. Hide the log column if there are no logs available

8673fe1

Fix a few minor style / formatting nits.

d190936

Reset listener after each test Don't null listener out at end of main().

ksakellis force-pushed the kostas-spark-2450 branch from df5ce42 to d190936 Compare February 6, 2015 02:05

asfgit closed this in 32e964c Feb 6, 2015

andrewor14 mentioned this pull request Feb 12, 2015

[SPARK-5736][Web UI]Add executor log url to Executors page on Yarn #4529

Closed

JoshRosen reviewed Mar 5, 2015
View reviewed changes

JoshRosen mentioned this pull request Mar 5, 2015

[SPARK-6175] Fix standalone executor log links when ephemeral ports or SPARK_PUBLIC_DNS are used #4903

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SPARK-2450 Adds executor log links to Web UI #3486

SPARK-2450 Adds executor log links to Web UI #3486

ksakellis commented Nov 26, 2014

AmplabJenkins commented Nov 26, 2014

ksakellis commented Nov 26, 2014

andrewor14 commented Nov 30, 2014

SparkQA commented Nov 30, 2014

SparkQA commented Nov 30, 2014

AmplabJenkins commented Nov 30, 2014

sryza Dec 1, 2014

sryza commented Dec 1, 2014

SparkQA commented Dec 2, 2014

SparkQA commented Dec 2, 2014

SparkQA commented Dec 2, 2014

AmplabJenkins commented Dec 2, 2014

SparkQA commented Dec 2, 2014

AmplabJenkins commented Dec 2, 2014

vanzin Dec 8, 2014

vanzin Dec 8, 2014

vanzin Dec 8, 2014

andrewor14 commented Dec 9, 2014

ksakellis commented Dec 9, 2014

andrewor14 Dec 9, 2014

SparkQA commented Feb 4, 2015

AmplabJenkins commented Feb 4, 2015

SparkQA commented Feb 4, 2015

AmplabJenkins commented Feb 4, 2015

JoshRosen commented Feb 5, 2015

SparkQA commented Feb 6, 2015

SparkQA commented Feb 6, 2015

AmplabJenkins commented Feb 6, 2015

JoshRosen commented Feb 6, 2015

SparkQA commented Feb 6, 2015

SparkQA commented Feb 6, 2015

AmplabJenkins commented Feb 6, 2015

JoshRosen commented Feb 6, 2015

JoshRosen Mar 5, 2015

SPARK-2450 Adds executor log links to Web UI #3486

SPARK-2450 Adds executor log links to Web UI #3486

Conversation

ksakellis commented Nov 26, 2014

AmplabJenkins commented Nov 26, 2014

ksakellis commented Nov 26, 2014

andrewor14 commented Nov 30, 2014

SparkQA commented Nov 30, 2014

SparkQA commented Nov 30, 2014

AmplabJenkins commented Nov 30, 2014

sryza Dec 1, 2014

Choose a reason for hiding this comment

sryza commented Dec 1, 2014

SparkQA commented Dec 2, 2014

SparkQA commented Dec 2, 2014

SparkQA commented Dec 2, 2014

AmplabJenkins commented Dec 2, 2014

SparkQA commented Dec 2, 2014

AmplabJenkins commented Dec 2, 2014

vanzin Dec 8, 2014

Choose a reason for hiding this comment

vanzin Dec 8, 2014

Choose a reason for hiding this comment

vanzin Dec 8, 2014

Choose a reason for hiding this comment

andrewor14 commented Dec 9, 2014

ksakellis commented Dec 9, 2014

andrewor14 Dec 9, 2014

Choose a reason for hiding this comment

SparkQA commented Feb 4, 2015

AmplabJenkins commented Feb 4, 2015

SparkQA commented Feb 4, 2015

AmplabJenkins commented Feb 4, 2015

JoshRosen commented Feb 5, 2015

SparkQA commented Feb 6, 2015

SparkQA commented Feb 6, 2015

AmplabJenkins commented Feb 6, 2015

JoshRosen commented Feb 6, 2015

SparkQA commented Feb 6, 2015

SparkQA commented Feb 6, 2015

AmplabJenkins commented Feb 6, 2015

JoshRosen commented Feb 6, 2015

JoshRosen Mar 5, 2015

Choose a reason for hiding this comment