codope commented on code in PR #18988:
URL: https://github.com/apache/hudi/pull/18988#discussion_r3687534826
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/callback/common/HoodieWriteCommitCallbackMessage.java:
##########
@@ -69,10 +73,91 @@ public class HoodieWriteCommitCallbackMessage implements
Serializable {
*/
private final Option<Map<String, String>> extraMetadata;
+ /**
+ * Previous base file paths keyed by fileId, derived lazily from {@link
#hoodieWriteStat} and
+ * the {@link BaseFileOnlyView} handed over by the write client, so that
callback
+ * implementations don't have to rebuild a view themselves. Empty for
inserts and for
+ * callers that don't supply a view.
+ *
+ * <p>Resolution is deferred until the first {@link #getPrevFilePaths()}
call: a callback
+ * that never reads the previous paths pays nothing (no FileSystemView
access). Transient
+ * because it captures a FileSystemView supplier, which is not serializable
- these paths
+ * are JVM-local derived state, so a message restored from Java
serialization reports none.
+ * Excluded from the generated getters so the {@link Lazy} wrapper never
leaks into JSON.
+ */
+ @Getter(AccessLevel.NONE)
+ private final transient Lazy<Map<String, PrevFilePaths>> prevFilePaths;
+
+ /**
+ * Free-form context that producers can attach for downstream callback
consumers.
+ * The OSS write client populates this as empty; specialized callsites or
wrappers
+ * may populate it with whatever context their callbacks need.
+ */
+ private final Map<String, String> extraContext;
+
+ public HoodieWriteCommitCallbackMessage(String commitTime,
+ String tableName,
+ String basePath,
+ List<HoodieWriteStat>
hoodieWriteStat,
+ Option<String> commitActionType,
+ Option<Map<String, String>>
extraMetadata,
+ Supplier<BaseFileOnlyView>
fsViewSupplier,
+ Map<String, String> extraContext) {
+ this.commitTime = commitTime;
+ this.tableName = tableName;
+ this.basePath = basePath;
+ this.hoodieWriteStat = hoodieWriteStat;
+ this.commitActionType = commitActionType;
+ this.extraMetadata = extraMetadata;
+ this.prevFilePaths = Lazy.lazily(() ->
HoodieWriteCommitCallbackUtil.resolvePrevFilePaths(
+ hoodieWriteStat, fsViewSupplier == null ? null :
fsViewSupplier.get()));
+ this.extraContext = extraContext;
+ }
+
public HoodieWriteCommitCallbackMessage(String commitTime,
String tableName,
String basePath,
List<HoodieWriteStat>
hoodieWriteStat) {
- this(commitTime, tableName, basePath, hoodieWriteStat, Option.empty(),
Option.empty());
+ this(commitTime, tableName, basePath, hoodieWriteStat, Option.empty(),
Option.empty(),
+ null, Collections.emptyMap());
+ }
+
+ public HoodieWriteCommitCallbackMessage(String commitTime,
+ String tableName,
+ String basePath,
+ List<HoodieWriteStat>
hoodieWriteStat,
+ Option<String> commitActionType,
+ Option<Map<String, String>>
extraMetadata) {
+ this(commitTime, tableName, basePath, hoodieWriteStat, commitActionType,
extraMetadata,
+ null, Collections.emptyMap());
+ }
+
+ /**
+ * Returns the previous base file paths keyed by fileId, resolving them from
the file-system
+ * view on first access. A consumer that never calls this triggers no
FileSystemView lookup.
+ * Never null: empty when no view was supplied, when the commit only
inserted, or when this
+ * message was restored from Java serialization.
+ */
+ public Map<String, PrevFilePaths> getPrevFilePaths() {
Review Comment:
Thanks @danny0405. I'll take this and push the two-field version. One thing
to correct though:
> it seems the Ser/De customization is unavoidable. The PR description
already claims this behavior, but the current implementation does not provide
it...
That PR description was outdated. Nothing actually forces the behaviour. All
three built-in callbacks (HTTP, Kafka, Pulsar) go through
`convertToJsonString`, and Jackson invokes `getPrevFilePaths()` while
generating the payload, so the JSON path resolves the paths either way. Nothing
in the repo Java-serializes the message, and `prevFilePaths` is new in this PR,
so no consumer can be depending on it yet.
But, i agree with your choice. The class is a `@PublicAPIClass` and
implements `Serializable`, so a custom callback that ships the message across a
JVM boundary would silently get an empty map rather than an error.
--
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]
