[HUDI-9332] Pluggable Table Format Support with native Integration by bvaradar · Pull Request #13216 · apache/hudi

bvaradar · 2025-04-24T00:42:13Z

Change Logs

[HUDI-9332] Pluggable Table Format Support with native Integration

Includes base interface for pluggable table format
Includes native hudi integration for pluggable table format
Includes the MDT side integration for puggability.

Impact

Initial Implementation of RFC - 93

Risk level (write none, low medium or high below)

none

Documentation Update

none

Contributor's checklist

Read through contributor's guide
Change Logs and Impact were stated clearly
Adequate tests were added if applicable
CI passed

Copilot

Pull Request Overview

This PR implements pluggable table format support with native integration, enabling the system to use a configurable table format for various timeline operations instead of the fixed timeline layout. Key changes include:

Replacing direct usage of timeline layout with the pluggable table format interface across multiple modules.
Introducing new methods and fields (e.g. in HoodieTableMetaClient and HoodieTableConfig) to support native table format integration.
Updating tests and client code to exercise the new table format behavior.

Reviewed Changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/BaseHoodieTimeline.java	Added helper method to retrieve instants via the file system using the metaClient.
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java	Integrated pluggable table format support and introduced a native active timeline accessor.
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java	Added configuration properties and a method to instantiate a pluggable table format.
hudi-common/src/main/java/org/apache/hudi/common/PluggableTableFormat.java & HudiPluggableTableFormat.java	Defined and implemented the pluggable table format interface.
Various client modules and tests	Replaced timeline layout factory calls with table format factory calls to support the new integration.

Comments suppressed due to low confidence (2)

hudi-common/src/main/java/org/apache/hudi/common/HoodieTableConfig.java:739

The ReflectionUtils.loadClass call currently does not capture or use the loaded class instance, as the method always returns a new HudiPluggableTableFormat. Consider instantiating and returning the loaded class instance if a supplementary table format class is provided.

ReflectionUtils.loadClass(getSupplementaryTableFormatClassName(), new Class[] {TimelineLayoutVersion.class}, new Object[] {layoutVersion});

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:121

[nitpick] Consider simplifying the lambda expression by using flatMap directly on the streams from each action to collect archived instants. This will enhance code readability and reduce unnecessary intermediate collection steps.

List<HoodieInstant> archivedInstants = instantsToArchive.stream()
            .map(action -> Stream.concat(action.getCompletedInstants().stream(), action.getPendingInstants().stream()).collect(Collectors.toList()))
            .flatMap(Collection::stream).collect(Collectors.toList());

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java

...n/src/main/java/org/apache/hudi/table/action/rollback/CopyOnWriteRollbackActionExecutor.java

hudi-common/src/main/java/org/apache/hudi/common/PluggableTableFormat.java

hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataFactory.java

hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/StreamSync.java

vinothchandar

Made a pass. Have a directional comment which I feel may be cross cutting on the PR

...nt/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v1/TimelineArchiverV1.java

hudi-common/src/main/java/org/apache/hudi/common/HudiPluggableTableFormat.java

hudi-common/src/main/java/org/apache/hudi/common/PluggableTableFormat.java

hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java

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

hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/StreamSync.java

geserdugarov · 2025-05-30T08:10:34Z

I don't understand costs in terms of performance, that we should pay for this change. There are a lot of work that was already done, and there is still a lot to do down the road. From my point of view, performance is a critical parameter for users when we are talking about Big Data processing.

Is there any benchmark results for this change?

geserdugarov · 2025-05-30T09:07:14Z

Another question, what will happen, if we wrote Hudi metadata successfully, but failed during writing of Iceberg or Delta metadata? How we will process this case, when Hudi table is fine, but interoperability support is broken at some commit?

hudi-common/src/main/java/org/apache/hudi/common/HudiPluggableTableFormat.java

hudi-common/src/main/java/org/apache/hudi/common/PluggableTableFormat.java

vinothchandar

Left some comments (some nts: note to self) and some legit to check.

If you can rebase, address as much of these as possible - then I can take another full pass.

Please feel free to resolve code review comments that are addressed.

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v1/TimelineArchiverV1.java

...nt/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java

...n/src/main/java/org/apache/hudi/table/action/rollback/CopyOnWriteRollbackActionExecutor.java

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java

hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java

hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java

hudi-common/src/main/java/org/apache/hudi/common/NativeTableFormat.java

hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/source/stats/FileStatsIndex.java

...spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala

vinothchandar

Abstractions look good. Left a bunch of comments for me check, shepherd and land. I can make those myself.

vinothchandar · 2025-06-23T21:05:53Z

...n/src/main/java/org/apache/hudi/table/action/rollback/CopyOnWriteRollbackActionExecutor.java

      table.getMetaClient().reloadActiveTimeline();
    }

+    // If instant is inflight but marked as completed in native format, delete the completed instant from storage.


nts: to review closely

Is this a bug fix? how could this happen?

When committing -> we commit to native timeline first (A), then plugged-in tableformat (B)

instantToRollback.isInflight() can be true, if A happened, but we failed before B. This is fixing that.

I ll make this limited to cases where something else is plugged in

yeah, let's avoid listing the timeline multiple times for reqular workflow when the external table format is not there.

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java

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

...mmon/src/main/java/org/apache/hudi/common/table/timeline/versioning/v1/ActiveTimelineV1.java

...mmon/src/main/java/org/apache/hudi/common/table/timeline/versioning/v2/ActiveTimelineV2.java

vinothchandar · 2025-06-23T21:12:58Z

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

   */
  HoodieInstant transitionClusterInflightToComplete(boolean shouldLock, HoodieInstant inflightInstant, HoodieReplaceCommitMetadata metadata);

+  HoodieInstant transitionClusterInflightToComplete(boolean shouldLock, HoodieInstant inflightInstant, HoodieReplaceCommitMetadata metadata, TableFormatCompletionAction tableFormatCompletionAction);


we should try and avoid the overload?

hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java

hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/StreamSync.java

vinothchandar · 2025-06-25T23:52:06Z

...mmon/src/main/java/org/apache/hudi/common/table/timeline/versioning/v2/ActiveTimelineV2.java

            metaClient.getStorage().exists(getInstantFileNamePath(fromInstantFileName)),
            "File " + getInstantFileNamePath(fromInstantFileName) + " does not exist!");
-        createCompleteFileInMetaPath(shouldLock, toInstant, metadata);
+        String completionTime = HoodieInstantTimeGenerator.formatDateBasedOnTimeZone(new Date(createCompleteFileInMetaPath(shouldLock, toInstant, metadata)));


nts: need to ensure this is consistent with our approach to time generation

I think this is fine. keeping this open.. and tracking.

Will get @the-other-tim-brown 's eyes on this next week.

The logic is correct but redundant, I will fix this.

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v1/TimelineArchiverV1.java

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java

hudi-common/src/main/java/org/apache/hudi/common/NativeTableFormat.java

danny0405

block on my final review.

vinothchandar · 2025-06-30T18:44:37Z

@hudi-bot run azure

vinothchandar · 2025-06-30T21:00:29Z

@hudi-bot run azure

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v1/TimelineArchiverV1.java

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java

vinothchandar · 2025-07-01T02:38:32Z

@geserdugarov sorry. missed your comments there.. does the RFC help address some of those

unblock it from myside first.

vinothchandar · 2025-07-01T15:03:32Z

hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/source/stats/FileStatsIndex.java


+  private void initMetaClient() {
+    if (this.metaClient == null) {
+      this.metaClient = StreamerUtil.createMetaClient(conf);


@danny0405 general question for you. is StreamerUtil the right place for such reusable helper methods. I would have expected sth like FlinkTableUtils or sth generic.

yeah, for most of the the utilities method we put it here. we also have other utilities, you can check all the other utility classes under the same package.

Address review comments Avoid local timezone conversion for completion time Handle rollback and savepoint. Add TableFormatCompletionAction functional interface. Fix bug in saveAsComplete. Use Consumer instead Add functional tests for TableFormat Fix check-style Fix renames Address comments from Vinoth Fix tests and improve test coverage Refactor FsUtils to take metaClient and remove overloaded methods

… lazy

vinothchandar

Pushed one last change.. Tests pass, then I am good with the PR.

@danny0405 to make a pass and shepherd/land this.

cc @bvaradar @vinishjail97

hudi-bot · 2025-07-02T04:16:11Z

CI report:

222d7b0 Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

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

geserdugarov · 2025-07-02T06:34:49Z

@geserdugarov sorry. missed your comments there.. does the RFC help address some of those

@vinothchandar, thanks for your reply. Unfortunately, the RFC doesn't address mentioned questions. My main concern is performance, I really hope we won’t lose it after these changes. Since there are no performance tests in the project, we’ll have to verify it manually.
Another issue is the increasing code complexity. It keeps growing, and it’s becoming harder and harder to provide a stable solution. I hope some progress will be done under HUDI-9054.

danny0405 · 2025-07-02T07:27:38Z

@geserdugarov what is the performance concern that it may induces in this PR? We should definitely fix it IMO, can you guide me the lines in this patch?

In general, all the changes in this path should not bring in performance issues, some hook/empty invocations shoud be fine and the cost could be negligible.

I agree we should limit the code complexity, it's the way we are evolving the project, we welcome new features always if it is valuable, then try to make the code in good shape as much as we can, but yeah, sometimes it is hard to give a elegant solution based on the amout of work it could take.

Appreciate your points.

geserdugarov · 2025-07-02T08:16:28Z

@geserdugarov what is the performance concern that it may induces in this PR? We should definitely fix it IMO, can you guide me the lines in this patch?

In general, all the changes in this path should not bring in performance issues, some hook/empty invocations shoud be fine and the cost could be negligible.

I agree we should limit the code complexity, it's the way we are evolving the project, we welcome new features always if it is valuable, then try to make the code in good shape as much as we can, but yeah, sometimes it is hard to give a elegant solution based on the amout of work it could take.

Appreciate your points.

@danny0405 , from quick checking of this patch, you're right, looks like there should be no performance issues. I only see that we started to pass HoodieTableMetaClient way more, and it's a Serializable class, which contains a lot of not transient objects. If there are any frequently called place in the code, where we will start to use Kryo SerDe of this class, (which I don't actually see) it could potentially decrease performance. But again, it's just a guessing, and it would be great to have some continuous performance monitoring to be sure in all big changes. I also will try to think about it in my spare time.

danny0405 · 2025-07-02T11:03:35Z

looks like there should be no performance issues. I only see that we started to pass HoodieTableMetaClient way more

@geserdugarov I also noticed this change, in most of the changes, the context already contains a meta client, and we just need it's light-weight member like "table format", the other logic are almost the same before the patch.

vinothchandar · 2025-07-02T14:49:26Z

it's just a guessing, and it would be great to have some continuous performance monitoring to be sure in all big changes.

+1. this is a great idea in general.. for this PR, we had a bunch of conversations around this. and @bvaradar explicitly assured me and ensured that we ll not incur overheads on native path.

@danny0405 I plugged the one issue I saw around extra timeline calls. CI is green. I'll land this in few hours, until I hear otherwise.

danny0405

+1

…pache#13216) * introduces a new table option 'hoodie.table.format'; * hoodie's native format just triggers nothing on core actions like #commit, #archive, #rollback, #clean, etc. --------- Co-authored-by: vinoth chandar <vinoth@apache.org> Co-authored-by: danny0405 <yuzhao.cyz@gmail.com>

vinothchandar self-assigned this Apr 25, 2025

vinothchandar requested review from Copilot and vinothchandar April 25, 2025 16:49

Copilot AI reviewed Apr 25, 2025

View reviewed changes

github-actions bot added the size:L PR with lines of changes in (300, 1000] label Apr 25, 2025

vinishjail97 reviewed Apr 26, 2025

View reviewed changes

vinothchandar requested changes May 7, 2025

View reviewed changes

bvaradar mentioned this pull request Jun 2, 2025

[722] Implement iceberg versions for TableFormat and HoodieTableMetadata apache/incubator-xtable#723

Open

vinishjail97 reviewed Jun 6, 2025

View reviewed changes

hudi-common/src/main/java/org/apache/hudi/common/HudiPluggableTableFormat.java Show resolved Hide resolved

vinishjail97 reviewed Jun 6, 2025

View reviewed changes

hudi-common/src/main/java/org/apache/hudi/common/PluggableTableFormat.java Show resolved Hide resolved

github-actions bot added size:XL PR with lines of changes > 1000 and removed size:L PR with lines of changes in (300, 1000] labels Jun 6, 2025

balaji-varadarajan-ai force-pushed the pluggable-tf-interface branch from e402ce7 to 9a2d3d6 Compare June 6, 2025 16:50

vinothchandar requested changes Jun 9, 2025

View reviewed changes

balaji-varadarajan-ai force-pushed the pluggable-tf-interface branch 5 times, most recently from a7b6ae7 to a3477dc Compare June 18, 2025 00:37

vinothchandar reviewed Jun 23, 2025

View reviewed changes

vinothchandar force-pushed the pluggable-tf-interface branch from a3477dc to 62e911a Compare June 25, 2025 23:45

vinothchandar reviewed Jun 25, 2025

View reviewed changes

danny0405 reviewed Jun 26, 2025

View reviewed changes

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v1/TimelineArchiverV1.java Outdated Show resolved Hide resolved

danny0405 reviewed Jun 26, 2025

View reviewed changes

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java Outdated Show resolved Hide resolved

danny0405 reviewed Jun 26, 2025

View reviewed changes

hudi-common/src/main/java/org/apache/hudi/common/NativeTableFormat.java Outdated Show resolved Hide resolved

danny0405 previously requested changes Jun 27, 2025

View reviewed changes

vinothchandar force-pushed the pluggable-tf-interface branch 2 times, most recently from 1b4151b to c146e82 Compare June 28, 2025 00:40

vinothchandar force-pushed the pluggable-tf-interface branch from c146e82 to 3fc8cbf Compare June 28, 2025 17:18

danny0405 reviewed Jul 1, 2025

View reviewed changes

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v1/TimelineArchiverV1.java Outdated Show resolved Hide resolved

danny0405 reviewed Jul 1, 2025

View reviewed changes

...t-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java Show resolved Hide resolved

vinothchandar reviewed Jul 1, 2025

View reviewed changes

bvaradar and others added 9 commits July 1, 2025 08:13

Rebasing

4f3fabf

Wiring in the property in few more places

da763c8

Fix test failure

daa22ca

Renaming the interface to HoodieTableFormat

fb0e50b

fix meta client creation in file index

c23dcbd

Minor changes to RFC-93 based on PR

6dc7f2c

fix the file stats index meta client, fix the archived instants to be…

cf93d8d

… lazy

Minor test/code cleanup

59cf106

vinothchandar force-pushed the pluggable-tf-interface branch from 84c72fd to 59cf106 Compare July 1, 2025 22:37

Limiting handling to cases where non-native table format is used

fc83977

vinothchandar approved these changes Jul 2, 2025

View reviewed changes

fix the reduadant completion time computation

222d7b0

danny0405 approved these changes Jul 3, 2025

View reviewed changes

danny0405 merged commit 8a5163b into apache:master Jul 3, 2025
120 of 122 checks passed

Conversation

bvaradar commented Apr 24, 2025 • edited by vinothchandar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Logs

Impact

Risk level (write none, low medium or high below)

Documentation Update

Contributor's checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vinothchandar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

geserdugarov commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geserdugarov commented May 30, 2025

Uh oh!

Uh oh!

Uh oh!

vinothchandar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vinothchandar 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!

danny0405 Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bvaradar commented Apr 24, 2025 •

edited by vinothchandar

Loading

geserdugarov commented May 30, 2025 •

edited

Loading

danny0405 Jul 2, 2025 •

edited

Loading

vinothchandar Jul 1, 2025 •

edited

Loading

geserdugarov commented Jul 2, 2025 •

edited

Loading

geserdugarov commented Jul 2, 2025 •

edited

Loading

danny0405 commented Jul 2, 2025 •

edited

Loading