[CELEBORN-2415] Add Celeborn Spark UI extension - #3793
Conversation
4f8bad3 to
68fc2f2
Compare
7a9e389 to
47f3676
Compare
47f3676 to
e053012
Compare
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a new Spark UI / History Server extension module that surfaces Celeborn-related shuffle summary metrics and spark.celeborn.* configuration in a dedicated “Celeborn” tab.
Changes:
- Introduces new module
client-spark/spark-3-uiwith Spark plugin + History Server plugin, status-store access, and UI rendering. - Adds a Spark 3 vs Spark 4 servlet (
javaxvsjakarta) bridge via profile-selected source roots. - Wires the new UI module into root build and shaded Spark client artifacts.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Adds the new UI module to Spark build profiles and introduces servlet.source.dir to select Spark3/Spark4 servlet bridge sources. |
| client-spark/spark-3-ui/pom.xml | New module POM with Spark dependency + servlet APIs and build-helper source-root selection. |
| client-spark/spark-4-shaded/pom.xml | Includes the new UI artifact in the Spark 4 shaded assembly dependencies. |
| client-spark/spark-3-shaded/pom.xml | Includes the new UI artifact in the Spark 3 shaded assembly dependencies. |
| client-spark/spark-3-ui/src/main/scala/org/apache/spark/shuffle/celeborn/CelebornPlugin.scala | Adds SparkPlugin entrypoint for live UI, registers listener and attaches tab. |
| client-spark/spark-3-ui/src/main/scala/org/apache/spark/shuffle/celeborn/CelebornHistoryServerPlugin.scala | Adds SHS plugin SPI implementation to replay metrics and attach the tab. |
| client-spark/spark-3-ui/src/main/scala/org/apache/spark/shuffle/celeborn/CelebornListener.scala | Listener aggregates task metrics and persists rollups + properties into KVStore. |
| client-spark/spark-3-ui/src/main/scala/org/apache/spark/shuffle/celeborn/CelebornStatusStore.scala | Status-store accessors for aggregated metrics and celeborn properties. |
| client-spark/spark-3-ui/src/main/scala/org/apache/spark/shuffle/celeborn/ui/CelebornUITab.scala | Defines the “Celeborn” tab and registers the page. |
| client-spark/spark-3-ui/src/main/scala/org/apache/spark/shuffle/celeborn/ui/CelebornShufflePage.scala | Renders summary metrics + collapsible properties table. |
| client-spark/spark-3-ui/src/main/scala-spark3/org/apache/spark/shuffle/celeborn/ui/SparkServletBridge.scala | Spark 3 servlet type alias (javax.servlet). |
| client-spark/spark-3-ui/src/main/scala-spark4/org/apache/spark/shuffle/celeborn/ui/SparkServletBridge.scala | Spark 4 servlet type alias (jakarta.servlet). |
| client-spark/spark-3-ui/src/main/resources/META-INF/services/org.apache.spark.status.AppHistoryServerPlugin | Registers the SHS plugin via SPI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| private[celeborn] class CelebornPropertiesUIData( | ||
| val info: Seq[(String, String)]) { | ||
|
|
||
| @JsonIgnore | ||
| @KVIndex | ||
| def id: String = classOf[CelebornPropertiesUIData].getName | ||
| } |
There was a problem hiding this comment.
Not necessary. Verified end-to-end that the properties table renders correctly in both live UI and SHS replay. Spark's KVStore uses Jackson with the Scala
| if (statusStore.hasData()) { | ||
| new CelebornUITab(statusStore, ui) | ||
| } |
There was a problem hiding this comment.
Fair point on the live/SHS inconsistency. Fixed by attaching the tab unconditionally in SHS (matching the live UI, which always attaches once spark.plugins=CelebornPlugin is set) and removing the now-unused hasData().
| private val totalTaskDurationMs = new AtomicLong(0L) | ||
|
|
||
| private val lastUpdateTimestamp = new AtomicLong(-1L) | ||
| private val updateIntervalMillis = 5000L |
There was a problem hiding this comment.
Reasonable for later, but intentionally a fixed default for the MVP. We can expose spark.celeborn.ui.flushIntervalMs in a follow-up once we see real cadence
needs.
| val last = lastUpdateTimestamp.get() | ||
| if (!force && (last != -1L && (now - last) < updateIntervalMillis)) { | ||
| return | ||
| } | ||
| if (lastUpdateTimestamp.compareAndSet(last, now) || force) { | ||
| flushAggregations() | ||
| } |
There was a problem hiding this comment.
Impact is negligible in practice: force=true is only reached from onApplicationEnd, which fires once at app end with no concurrent onTaskEnd. No extra flushes follow it. Leaving as-is.
| <dependency> | ||
| <groupId>jakarta.servlet</groupId> | ||
| <artifactId>jakarta.servlet-api</artifactId> | ||
| <scope>provided</scope> | ||
| </dependency> |
There was a problem hiding this comment.
Intentional. A single module pom with ${servlet.source.dir} selecting the source root keeps the build simple; both servlet APIs are provided and only the
one matching the selected source root is ever referenced at compile time. Splitting into per-profile dependencies would duplicate the dependency block
across every Spark profile without real benefit.
| <module>client-spark/spark-3</module> | ||
| <module>client-spark/spark-3-columnar-common</module> | ||
| <module>client-spark/spark-3-columnar-shuffle</module> | ||
| <module>client-spark/spark-3-ui</module> |
There was a problem hiding this comment.
The name follows the existing celeborn-client-spark-3 convention in this repo, which also serves Spark 4. Renaming would diverge from that. Keeping spark-3-ui for consistency.
| <span class="collapse-aggregated-celebornProperties collapse-table" | ||
| onClick="collapseTable('collapse-aggregated-celebornProperties', | ||
| 'aggregated-celebornProperties')"> | ||
| <h4> | ||
| <span class="collapse-table-arrow arrow-open"></span> | ||
| <a>Celeborn Properties</a> | ||
| </h4> | ||
| </span> | ||
| <div class="aggregated-celebornProperties collapsible-table"> |
There was a problem hiding this comment.
Following Spark's own UI convention here — Spark uses camelCase collapse identifiers (e.g. collapse-aggregated-allActiveStages). The collapseTable() JS
accepts arbitrary ids as long as the collapse-* prefix and the target class match, which they do. No change.
setupUI() no longer gates on hasData(); the Celeborn tab is now always attached in the History Server, matching the live UI behavior where the tab is attached unconditionally once spark.plugins=CelebornPlugin is set. Removes the now-unused CelebornStatusStore.hasData().
c0cd150 to
18cf1d1
Compare
|
ping @Kalvin2077 |
What changes were proposed in this pull request?
Add a new module
celeborn-client-spark-3-ui— a Spark UI extension that renders a "Celeborn" tab in both the live Spark UI and the History Server (enabled viaspark.plugins=org.apache.spark.shuffle.celeborn.CelebornPlugin, supports Spark 3.x and 4.x).This is an initial version that lays the foundation; it intentionally starts minimal. The page shows:
TaskMetricsspark.celeborn.*configsThe skeleton is:
CelebornPlugin(live UI) /CelebornHistoryServerPlugin(SHS, viaAppHistoryServerPluginSPI) → listener aggregates into SparkKVStore→CelebornStatusStore→CelebornUITabrenders; ajavax/jakartaservlet bridge covers Spark 3.x and 4.x. This plugin/store/render pipeline is designed so that fine-grained Celeborn client metrics can be added as collapsible sections in follow-up PRs:Why are the changes needed?
Spark UI already shows stage/task-level shuffle metrics, but nothing
Celeborn-specific: which workers served the data, push/chunk latencies, congestion and split events are invisible there, while Celeborn's own metrics sit in cluster-level Grafana dashboards that cannot be attributed back to a specific application. This tab closes that gap by putting per-application Celeborn shuffle information inside the Spark UI and History Server, alongside the job's stages and tasks.Does this PR introduce any user-facing change?
Yes.
Setting
spark.plugins=org.apache.spark.shuffle.celeborn.CelebornPlugin(with the extension jar on the classpath) adds a "Celeborn" tab to the Spark UI and History Server. Disabled by default; no effect otherwise.Does this PR resolve a correctness bug?
No.
How was this patch tested?
Manual end-to-end verification: