zaynt4606 commented on code in PR #3870:
URL: https://github.com/apache/celeborn/pull/3870#discussion_r4266170067
##########
common/pom.xml:
##########
@@ -192,6 +192,56 @@
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>buildnumber-maven-plugin</artifactId>
+ <configuration>
+
<outputDirectory>${project.build.directory}/generated/build-metadata</outputDirectory>
+ <outputName>build.properties</outputName>
+ <revisionPropertyName>git.revision</revisionPropertyName>
Review Comment:
[P1] Allow builds when Git revision metadata is unavailable
This new goal makes Git metadata mandatory for building `celeborn-common`.
Celeborn's source release is produced with `git archive`, so its extracted
sources do not contain `.git`. I reproduced a build failure from an archive of
this revision with:
```text
mvn -pl common -am -Dmaven.test.skip=true package
```
The build fails in `buildnumber-maven-plugin:1.4:create-metadata` with
`NullPointerException` at `CreateMetadataMojo.java:188`. The currently failing
[C++ integration
CI](https://github.com/apache/celeborn/actions/runs/38397241592/job/115295484286)
reports the same exception in this newly added goal.
Could we configure `revisionOnScmFailure` (for example, `N/A`, matching the
SBT fallback) and add validation for a source archive without Git metadata?
Adding `-Dmaven.buildNumber.revisionOnScmFailure=N/A` made the same
archived-source package build pass.
##########
client-spark/spark-3-ui/src/main/scala/org/apache/spark/shuffle/celeborn/CelebornPlugin.scala:
##########
@@ -49,10 +50,29 @@ private class CelebornDriverPlugin extends DriverPlugin
with Logging {
logInfo("Initializing CelebornDriverPlugin...")
this.sc = sc
val kvStore = sc.statusStore.store
+ writeBuildInfo(kvStore)
new CelebornListener(kvStore, sc.conf).register(sc)
java.util.Collections.emptyMap[String, String]()
}
+ /**
+ * Persists the build info of the celeborn-client bundled with this
application into the
+ * KVStore so that it survives application end and is available on History
Server replay,
+ * where the Spark UI "Celeborn" tab renders it as "Celeborn Client Build
Information".
+ */
+ private def writeBuildInfo(kvStore: KVStore): Unit = {
+ val buildInfo = new CelebornBuildInfoUIData(Seq(
+ "Version" -> org.apache.celeborn.ProjectConstants.VERSION,
+ "Commit Id" -> org.apache.celeborn.ProjectConstants.getGitCommitId,
+ "Revision" -> org.apache.celeborn.ProjectConstants.REVISION))
+ try {
+ kvStore.write(buildInfo)
Review Comment:
[P2] Record build information in the event log so History Server can restore
it
Writing to the Driver's KVStore does not persist this entry in the
application's event log. History Server constructs a separate store during
replay, and neither `CelebornHistoryServerPlugin` nor `CelebornListener`
restores `CelebornBuildInfoUIData`, so the new table is empty there despite the
documented replay support.
I verified this with a real Spark 3.5.1 application using `CelebornPlugin`
and event logging: the live store contained all 3 build-info entries. Replaying
that application's log with `ReplayListenerBus` and the actual
`CelebornHistoryServerPlugin` listeners produced `extensionEnabled=true` but 0
build-info entries.
Could we record the application's build information in a replayable event or
environment properties, reconstruct it in the replay listener, and add an
event-log round-trip test? The restored values need to come from the
application, rather than the client artifact installed on History Server.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
