[SPARK-44153][CORE][UI] Support Heap Histogram column in Executors tab

[dongjoon-hyun]

What changes were proposed in this pull request?

This PR aims to support Heap Histogram column in Executor tab.

Why are the changes needed?

Like Thread Dump column, this is very helpful when we analyze executor live JVM status.

Does this PR introduce any user-facing change?

Yes, but this is a new column and we provide spark.ui.heapHistogramEnabled configuration like spark.ui.threadDumpsEnabled.

How was this patch tested?

Manual review.

[dongjoon-hyun]

Hi, @viirya . This is the Heap Histogram PR. Could you review this when you have some time?

[dongjoon-hyun]

Java 8 JRE doesn't have jmap. For Java 8 JDK users, they are able this manually.

[dongjoon-hyun]

Here is the answer for jmap question, @viirya .

[viirya]

Hmm, what if executor node doesn't have jmap installed?

[dongjoon-hyun]

All Java 11+ has jmap because they are JDK~

[viirya]

If we get error from executing it, can we get the error back?

executeAndGetOutput seems can handle it?

[dongjoon-hyun]

Which error do you have in your mind? At this layer, we don't handle it like getThreadDump. For example, getThreadDump can throw SecurityException and UnsupportedOperationException, but we ignore them.

[dongjoon-hyun]

The error message will go to stderr of Executor log.

[dongjoon-hyun]

I took a look at executeAndGetOutput, but it's the same. It throws SparkException instead of get the error back.

spark/core/src/main/scala/org/apache/spark/util/Utils.scala

Lines 1378 to 1381 in 7398e93

	if (exitCode != 0) {
	logError(s"Process $command exited with code $exitCode: $output")
	throw new SparkException(s"Process $command exited with code $exitCode")
	}

[pan3793]

should it respect JAVA_HOME then PATH?

[dongjoon-hyun]

Like you pointed, there is no problem to run jmap, isn't it, @pan3793 ? IIRC, jmap is just CLI command without any format change. Especially, if you are saying JDKs here.

[pan3793]

I suppose the $JAVA_HOME/bin/jmap(in above case, should be /opt/openjdk-11/bin/jmap instead of /opt/openjdk-8/bin/jmap) should be used, instead of the one first present in PATH.

And if only JAVA_HOME is set, but no jmap in PATH, the invocation will fail. e.g.

The JDK was installed by TGZ, it was unarchived to /opt/openjdk-11 whithout exposing to PATH nor creating softlink to /usr/bin, then Spark executor proccess respect JAVA_HOME to find $JAVA_HOME/bin/java but the subprocess can not find jmap even it knows where JAVA_HOME is.

[dongjoon-hyun]

@pan3793 . Here, the contract on jmap is just one of available binary, not the same JDK's jmap.

However, I'm interested in this case. Could you make a small reproducer with Docker image?

Spark executor proccess respect JAVA_HOME to find $JAVA_HOME/bin/java but the subprocess can not find jmap even it knows where JAVA_HOME is.

[mridulm]

@dongjoon-hyun This is an issue - we should use $JAVA_HOME/bin/jmap (more specifically whatever comes from System.getProperty("java.home")), not the first jmap which happens to be in the PATH. It is common to override JAVA_HOME to specify the java version to be used explicitly (or even to not have jdk in the PATH at all).

Also, there is no compatibility gaurantees that I am aware of between different versions of jdk and jmap (for example, jdk11 jmap against jdk17 or vice versa) - if I missed any, please do let me know !

[dongjoon-hyun]

Got it. Let me take a look at that perspective, @mridulm and @pan3793 .

In the worst case, I'll limit this feature to K8s only environment because this was initially developed for K8s environment first.

[dongjoon-hyun]

In addition, we will ignore all irregular messages in addition to these three lines.

[viirya]

So the error will go to Executor log, and in UI it just doesn't show heap dump (empty)?

[dongjoon-hyun]

Yes, correct. We log and return None for Option[Array[String]]. So, the heap histogram UI will be empty. When the executor JVM is dead for some other reasons before driver notices it, it'll be handled in the same way.

spark/core/src/main/scala/org/apache/spark/SparkContext.scala

Lines 756 to 775 in 284029b

	private[spark] def getExecutorHeapHistogram(executorId: String): Option[Array[String]] = {
	try {
	if (executorId == SparkContext.DRIVER_IDENTIFIER) {
	Some(Utils.getHeapHistogram())
	} else {
	env.blockManager.master.getExecutorEndpointRef(executorId) match {
	case Some(endpointRef) =>
	Some(endpointRef.askSync[Array[String]](TriggerHeapHistogram))
	case None =>
	logWarning(s"Executor $executorId might already have stopped and " +
	"can not request heap histogram from it.")
	None
	}
	}
	} catch {
	case e: Exception =>
	logError(s"Exception getting heap histogram from executor $executorId", e)
	None
	}
	}

[dongjoon-hyun]

Oops. Sorry, @viirya . I forgot that ProcessHandle exists only from Java 9+. Let me convert this to Draft.

[dongjoon-hyun]

I converted back from Draft to normal. The last commit has Java8 compatible way.

- val pid = String.valueOf(ProcessHandle.current().pid()) + // From Java 9+, we can use 'ProcessHandle.current().pid()' + val pid = getProcessName().split("@").head 

[dongjoon-hyun]

Thank you for review and approval, @viirya !

[dongjoon-hyun]

Merged to master for Apache Spark 3.5.0.
Thank you, @viirya and @pan3793 .

[mridulm]

Log errors in invocation to executor logs instead of sending it to driver as response ?

[dongjoon-hyun]

That's totally different feature. This is designed as a part of Spark Driver UI, @mridulm .
Sorry, I misunderstood this request. Sure, I'll try.

[mridulm]

Use IOUtils.readLines or Source.getLines instead ?

[dongjoon-hyun]

Use IOUtils.readLines or Source.getLines instead ?

For this one, I was thinking about adding a new configuration to limit the results like Top 100 or Top 1000. I'll handle this with that new configuration together.

[mridulm]

nit: This reader is not closed and/or we are not doing waitFor on the process.