refactor: move checkpoint metadata lookup helper to hudi-common by suryaprasanna · Pull Request #18489 · apache/hudi

suryaprasanna · 2026-04-10T20:16:09Z

Describe the issue this Pull Request addresses

This PR moves the checkpoint metadata lookup helper into hudi-common so ingestion-related code can reuse the same timeline utility instead of keeping the logic in utilities-only code.

Summary and Changelog

Move checkpoint metadata lookup into TimelineUtils
Update streamer checkpoint lookup to delegate to the common helper
Keep support for both legacy and v2 checkpoint metadata keys in the utilities path
Remove the unused 2-key wrapper overload and keep a single varargs-based common helper entry point

Impact

No public API or storage format change. This is a refactoring that centralizes checkpoint metadata lookup logic for reuse and reduces duplication.

Risk Level

low

The change is limited to internal checkpoint metadata lookup flow and preserves the existing checkpoint-key handling in the streamer path.

Documentation Update

none

Contributor's checklist

Read through contributor's guide
Enough context is provided in the sections above
Adequate tests were added if applicable

yihua

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Style & Readability Review — a few simplification opportunities in the new helper method — the boolean flag check, if-else pattern, and unchecked cast could each be tightened slightly.

yihua · 2026-04-10T22:12:01Z

hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java

+      try {
+        HoodieCommitMetadata commitMetadata = HoodieCommitMetadata
+            .fromBytes(timeline.getInstantDetails(instant).get(), HoodieCommitMetadata.class);
+        boolean hasCheckpointMetadata = false;


🤖 nit: could you replace the for-loop with Arrays.stream(checkpointKeys).anyMatch(key -> !StringUtils.isNullOrEmpty(commitMetadata.getMetadata(key)))?

_{- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.}

yihua · 2026-04-10T22:12:01Z

hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java

+            break;
+          }
+        }
+        if (hasCheckpointMetadata) {


🤖 nit: could you simplify this if-else to a ternary: return hasCheckpointMetadata ? Option.of(...) : Option.empty();?

_{- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.}

yihua · 2026-04-10T22:12:01Z

hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java

+
+  public static Option<Pair<String, HoodieCommitMetadata>> getLatestInstantAndCommitMetadataWithValidCheckpointInfo(HoodieTimeline timeline,
+                                                                                                                    String... checkpointKeys) throws IOException {
+    return (Option<Pair<String, HoodieCommitMetadata>>) timeline.getReverseOrderedInstants().map(instant -> {


🤖 nit: add @SuppressWarnings("unchecked") above the method to document the intentional type cast.

_{- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.}

yihua

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for contributing! The refactoring direction is sound — centralizing checkpoint metadata lookup in hudi-common is a good idea. However, I noticed two behavioral differences between the old and new code that could cause subtle issues.

yihua · 2026-04-10T22:12:40Z

hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java

+      try {
+        HoodieCommitMetadata commitMetadata = HoodieCommitMetadata
+            .fromBytes(timeline.getInstantDetails(instant).get(), HoodieCommitMetadata.class);
+        boolean hasCheckpointMetadata = false;


🤖 The old code returned instant.toString() (which includes action type and state, e.g. 20230101000000__commit__COMPLETED), but this now returns instant.getTimestamp() (just the timestamp). Callers that relied on the full instant string representation would see different values. Could you verify this doesn't break any downstream consumers of the pair's left value?

_{- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.}

nope. the timeline passed in contains only completed instants.

🤖 Worth noting that while the timeline does only contain completed instants, the behavioral change here is about the format of the returned String, not the state of the instant. The old code returned instant.toString() (e.g. [20231215101530__deltacommit__COMPLETED]) while the new common helper returns instant.requestedTime() (e.g. 20231215101530). Downstream callers like getLatestInstantWithValidCheckpointInfo pass this String to WriteStatusValidator — could you confirm that those consumers are fine with receiving just the timestamp instead of the full toString representation?

@yihua
Ideally we should return the requested time since it is used in the error table. But since it is a behavior change and require testing. I will stick to the exiting logic of passing instant.toString.
As part of subsequent PR we can return Pair<HoodieInstant, HoodieCommitMetadata> instead of Pair<String, HoodieCommitMetadata>.
What do you think?
CC @nsivabalan

yihua · 2026-04-10T22:12:40Z

hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java

+
+  public static Option<Pair<String, HoodieCommitMetadata>> getLatestInstantAndCommitMetadataWithValidCheckpointInfo(HoodieTimeline timeline,
+                                                                                                                    String... checkpointKeys) throws IOException {
+    return (Option<Pair<String, HoodieCommitMetadata>>) timeline.getReverseOrderedInstants().map(instant -> {


🤖 The old code used timeline.readCommitMetadata(instant) while this uses HoodieCommitMetadata.fromBytes(...) directly. readCommitMetadata may handle additional cases (e.g., different commit metadata subtypes like HoodieReplaceCommitMetadata). Could you check whether bypassing readCommitMetadata causes any metadata to be deserialized incorrectly?

_{- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.}

This is the same code.

…d to hudi-common so that Ingestion can use this API for fetching checkpoints

yihua

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Style & Readability Review — Missing Javadoc for the new public method clarifying what the String in the returned Pair represents.

yihua · 2026-04-13T19:17:44Z

hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java

  }
+
+  @SuppressWarnings("unchecked")
+  public static Option<Pair<String, HoodieCommitMetadata>> getLatestInstantAndCommitMetadataWithValidCheckpointInfo(


🤖 nit: could you add Javadoc to this public method? It'd be especially helpful to document what the String in the returned Pair represents (e.g., instant.requestedTime()).

_{- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.}

Added javadoc for this method.

yihua

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for contributing! Clean refactoring that centralizes checkpoint metadata lookup. One behavioral difference to verify — the new common helper returns instant.requestedTime() where the old code returned instant.toString(), which changes the String value in the returned Pair for existing callers.

yihua · 2026-04-13T19:22:44Z

hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java

+    return (Option<Pair<String, HoodieCommitMetadata>>) timeline.getReverseOrderedInstants().map(instant -> {
+      try {
+        HoodieCommitMetadata commitMetadata = timeline.readCommitMetadata(instant);
+        boolean hasCheckpointMetadata = Arrays.stream(checkpointKeys)


🤖 The old code in StreamerCheckpointUtils returned instant.toString() (which includes action, state, completion time), but this new common helper returns instant.requestedTime() (just the timestamp). This silently changes the value returned to all existing callers. The downstream usage in StreamSync passes this string as committedInstantTime to error table writers, so requestedTime() is arguably more correct — but could you confirm this behavioral change is intentional?

_{- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.}

This is just a refactoring change no changes with respect to functionality.

yihua · 2026-04-13T19:22:45Z

hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java

+      try {
+        HoodieCommitMetadata commitMetadata = HoodieCommitMetadata
+            .fromBytes(timeline.getInstantDetails(instant).get(), HoodieCommitMetadata.class);
+        boolean hasCheckpointMetadata = false;


🤖 Worth noting that while the timeline does only contain completed instants, the behavioral change here is about the format of the returned String, not the state of the instant. The old code returned instant.toString() (e.g. [20231215101530__deltacommit__COMPLETED]) while the new common helper returns instant.requestedTime() (e.g. 20231215101530). Downstream callers like getLatestInstantWithValidCheckpointInfo pass this String to WriteStatusValidator — could you confirm that those consumers are fine with receiving just the timestamp instead of the full toString representation?

yihua

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for addressing the feedback! The behavioral concern I raised about requestedTime() vs toString() has been reverted back to instant.toString(), preserving the original behavior. The new Javadoc clearly documents the return type and contract. All prior findings from both my review and other reviewers have been addressed.

codecov-commenter · 2026-04-13T22:37:16Z

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.83%. Comparing base (fc7f303) to head (44a08aa).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...ache/hudi/common/table/timeline/TimelineUtils.java	75.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #18489   +/-   ##
=========================================
  Coverage     68.83%   68.83%           
- Complexity    28171    28187   +16     
=========================================
  Files          2459     2459           
  Lines        135095   135093    -2     
  Branches      16378    16372    -6     
=========================================
  Hits          92992    92992           
- Misses        34737    34738    +1     
+ Partials       7366     7363    -3

Flag	Coverage Δ
common-and-other-modules	`44.54% <77.77%> (+<0.01%)`	⬆️
hadoop-mr-java-client	`44.83% <0.00%> (-0.01%)`	⬇️
spark-client-hadoop-common	`48.43% <0.00%> (-0.01%)`	⬇️
spark-java-tests	`48.89% <0.00%> (-0.03%)`	⬇️
spark-scala-tests	`45.49% <0.00%> (+<0.01%)`	⬆️
utilities	`38.21% <77.77%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...di/utilities/streamer/StreamerCheckpointUtils.java	`92.06% <100.00%> (+3.02%)`	⬆️
...ache/hudi/common/table/timeline/TimelineUtils.java	`79.84% <75.00%> (-0.16%)`	⬇️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

hudi-bot · 2026-04-13T23:35:55Z

CI report:

44a08aa Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

github-actions bot added the size:S PR with lines of changes in (10, 100] label Apr 10, 2026

yihua reviewed Apr 10, 2026

View reviewed changes

suryaprasanna added 2 commits April 13, 2026 12:00

Moving getLatestInstantAndCommitMetadataWithValidCheckpointInfo metho…

4637d41

…d to hudi-common so that Ingestion can use this API for fetching checkpoints

Address review comments

ea8eb03

suryaprasanna force-pushed the checkpoint-method-refactoring branch from 950841d to ea8eb03 Compare April 13, 2026 19:06

yihua reviewed Apr 13, 2026

View reviewed changes

Address review comments

44a08aa

suryaprasanna force-pushed the checkpoint-method-refactoring branch from 2f10649 to 44a08aa Compare April 13, 2026 21:15

yihua reviewed Apr 13, 2026

View reviewed changes

Conversation

suryaprasanna commented Apr 10, 2026

Describe the issue this Pull Request addresses

Summary and Changelog

Impact

Risk Level

Documentation Update

Contributor's checklist

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Apr 13, 2026

Codecov Report

Uh oh!

hudi-bot commented Apr 13, 2026

CI report:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants