[HUDI-6334] Integrate logcompaction table service to metadata table and provides various bugfixes to metadata table #8900

suryaprasanna · 2023-06-07T20:59:10Z

Change Logs

Apart from the Logcompaction table service this change also contains other bug fixes to metadata table.
Following are the changes present.

Change cleaner policy to KEEP_LATEST_FILE_VERSIONS for metadata table
Added logs statements
Fix rollback of inflight deltacommits on metadata table
Add log statements to LogCompaction planner
Integrate logcompaction table service on metadata table
Include checks to block Logcompaction in presence of compaction with greater timestamp
Use instant range option for major and minor compaction on metadata table

Impact

The changes contains bug fixes to metadata table and new feature request to integrate with logcompaction. So, impact is around the metadata table.

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

Medium

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change

The config description must be updated if new configs are added or the default value of the configs are changed
Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the instruction to make
changes to the website.

Contributor's checklist

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

nsivabalan · 2023-06-09T05:19:15Z

...source/hudi-flink/src/test/java/org/apache/hudi/sink/TestStreamWriteOperatorCoordinator.java

    // test metadata table compaction
-    // write another 4 commits
-    for (int i = 1; i < 5; i++) {
+    // write another 9 commits to trigger compaction twice. Since default clean version to retain is 2.


@danny0405 : can you review changes in flink classes.

would rather prefer to set the clean version to retain as 1. Doing 9 commits is going to add to test time which is already quite high.

Why we need to change the clean strategy then?

Clean's retainFileVersions value is changed to 2 and it is actually hardcoded in HoodieMetadataWriteUtils and both Spark and Flink gets the writeConfig using this. So, this cannot be overridden.
The reason for making the change is to support restore of main table timeline for atleast 10 commits. Restore on main table also causes restore on metadata table. If the retainFileVersions is less than 2, then next cleaner job after compaction would remove previous file slice there by blocking restore on metadata table or loosing data.

The reason for making the change is to support restore

First of all, I'm confused why this change is related with restore ? The change is for MDT log compaction right? Can we address the restore issue in another PR ?

then next cleaner job after compaction would remove previous file slice there by blocking restore on metadata table or loosing data.

Does the new cleaning strategy solve the issue, even we keep at least 2 versions for each file group, it does not ensure we can restore to a long history commit.

Discussed offline, reverting this change.

...nt/src/test/java/org/apache/hudi/client/functional/TestHoodieClientOnMergeOnReadStorage.java

...di-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java

...ient/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataWriteUtils.java

hudi-spark-datasource/hudi-spark/src/main/java/org/apache/hudi/cli/ArchiveExecutorUtils.java

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandleFactory.java

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieCompactionConfig.java

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandleFactory.java

...di-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java

...ent/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataWriter.java

...nt/hudi-client-common/src/main/java/org/apache/hudi/table/action/compact/CompactHelpers.java

.../org/apache/hudi/table/action/compact/plan/generators/BaseHoodieCompactionPlanGenerator.java

...t/java/org/apache/hudi/client/functional/TestDataValidationCheckForLogCompactionActions.java

codope · 2023-06-09T05:49:10Z

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

+        });
+
+    // SOLO_COMMIT_TIMESTAMP is used during bootstrap so it is a valid timestamp
+    validInstantTimestamps.add(SOLO_COMMIT_TIMESTAMP);


But any MDT partition initialization instant timestamp is of the format SOLO_COMMIT_TIMESTAMP + 3-digit suffix, e.g. 00000000000000010 for FILES partition. How to account for the suffix?

yeah. good catch. we need to account for that

Yeah, updated it.

There are couple of other places too. I've fixed it in #8915

codope · 2023-06-09T05:50:25Z

...source/hudi-flink/src/test/java/org/apache/hudi/sink/TestStreamWriteOperatorCoordinator.java

    // test metadata table compaction
-    // write another 4 commits
-    for (int i = 1; i < 5; i++) {
+    // write another 9 commits to trigger compaction twice. Since default clean version to retain is 2.


would rather prefer to set the clean version to retain as 1. Doing 9 commits is going to add to test time which is already quite high.

...di-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java

...ient/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataWriteUtils.java

danny0405 · 2023-06-09T06:09:12Z

...source/hudi-flink/src/test/java/org/apache/hudi/sink/TestStreamWriteOperatorCoordinator.java

    // test metadata table compaction
-    // write another 4 commits
-    for (int i = 1; i < 5; i++) {
+    // write another 9 commits to trigger compaction twice. Since default clean version to retain is 2.


Why we need to change the clean strategy then?

...di-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java

.../org/apache/hudi/table/action/compact/plan/generators/BaseHoodieCompactionPlanGenerator.java

danny0405 · 2023-06-09T07:51:23Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieCompactionConfig.java

+      .key("hoodie.log.compaction.enable")
+      .defaultValue("false")
+      .sinceVersion("0.14")
+      .withDocumentation("By enabling log compaction through this config, log compaction will also get enabled for the metadata table.");


It seems only MDT uses the log compaction. If that is true, kind of think this belongs to a MDT config.

Log compaction can also be done on a main table, if it of MOR table type.

I think enable log compaction is still required so created another config in HoodieMetadataConfig class.

…various bugfixes to metadata table Following changes are also included as part of this commit: 1. Change cleaner policy to KEEP_LATEST_FILE_VERSIONS for metadata table 2. Added logs statements 3. Fix rollback of inflight deltacommits on metadata table 4. Add log statements to LogCompaction planner 5. Integrate logcompaction table service on metadata table 6. Include checks to block Logcompaction in presence of compaction with greater timestamp 7. Use instant range option for major and minor compaction on metadata table

…onfig/HoodieCompactionConfig.java Co-authored-by: Sagar Sumit <[email protected]>

…able/action/compact/CompactHelpers.java Co-authored-by: Sagar Sumit <[email protected]>

codope

LGTM. Can land once the CI is green.

codope · 2023-06-09T13:09:15Z

CI passed -https://dev.azure.com/apache-hudi-ci-org/apache-hudi-ci/_build/results?buildId=17706&view=results

hudi-bot · 2023-06-09T13:27:01Z

CI report:

4d26a4d Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

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

danny0405 · 2023-06-09T13:42:10Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandleFactory.java

      TaskContextSupplier taskContextSupplier,
      Option<BaseKeyGenerator> keyGeneratorOpt) {
+    LOG.info("Create update handle for fileId {} and partition path {} at commit {}", fileId, partitionPath, instantTime);
    if (table.requireSortedRecords()) {


Can we just remove this log, it's verbose.

danny0405 · 2023-06-09T13:42:15Z

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandleFactory.java

      TaskContextSupplier taskContextSupplier,
      Option<BaseKeyGenerator> keyGeneratorOpt) {
+    LOG.info("Get updateHandle for fileId {} and partitionPath {} at commit {}", fileId, partitionPath, instantTime);
    if (table.requireSortedRecords()) {


Can we just remove this log, it's verbose.

Sure, will change it to debug mode in subsequent patch.

suryaprasanna changed the title ~~Integrate logcompaction table service on metadata table and provides various bugfixes to metadata table~~ Integrate logcompaction table service to metadata table and provides various bugfixes to metadata table Jun 7, 2023

suryaprasanna changed the title ~~Integrate logcompaction table service to metadata table and provides various bugfixes to metadata table~~ [HUDI-6334] Integrate logcompaction table service to metadata table and provides various bugfixes to metadata table Jun 7, 2023

nsivabalan mentioned this pull request Jun 8, 2023

[HUDI-6200] Enhancements to the MDT for improving performance of larger indexes. #8684

Merged

4 tasks

suryaprasanna force-pushed the pr2-changes branch 2 times, most recently from fe74a9a to f9e3b8d Compare June 9, 2023 01:41

nsivabalan added release-0.14.0 priority:critical Production degraded; pipelines stalled labels Jun 9, 2023

nsivabalan assigned nsivabalan and codope Jun 9, 2023

nsivabalan reviewed Jun 9, 2023

View reviewed changes

danny0405 reviewed Jun 9, 2023

View reviewed changes

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandleFactory.java Outdated Show resolved Hide resolved

codope reviewed Jun 9, 2023

View reviewed changes

danny0405 reviewed Jun 9, 2023

View reviewed changes

...di-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java Outdated Show resolved Hide resolved

danny0405 reviewed Jun 9, 2023

View reviewed changes

.../org/apache/hudi/table/action/compact/plan/generators/BaseHoodieCompactionPlanGenerator.java Show resolved Hide resolved

danny0405 reviewed Jun 9, 2023

View reviewed changes

suryaprasanna and others added 10 commits June 9, 2023 15:46

Fix test failures

e586d49

Refactor

f8bb9f4

Refactoring

e839b4d

Addressed review comments

55f1e4a

Address review comments

e5c0315

Update hudi-client/hudi-client-common/src/main/java/org/apache/hudi/c…

e3dc34b

…onfig/HoodieCompactionConfig.java Co-authored-by: Sagar Sumit <[email protected]>

Update hudi-client/hudi-client-common/src/main/java/org/apache/hudi/t…

e40785a

…able/action/compact/CompactHelpers.java Co-authored-by: Sagar Sumit <[email protected]>

Remove duplicate log stmt

3bb5fe6

Address review comments

4d26a4d

codope force-pushed the pr2-changes branch from b700b9e to 4d26a4d Compare June 9, 2023 10:19

codope approved these changes Jun 9, 2023

View reviewed changes

apache deleted a comment from hudi-bot Jun 9, 2023

danny0405 reviewed Jun 9, 2023

View reviewed changes

codope merged commit 195ae3a into apache:master Jun 9, 2023

[HUDI-6334] Integrate logcompaction table service to metadata table and provides various bugfixes to metadata table #8900

[HUDI-6334] Integrate logcompaction table service to metadata table and provides various bugfixes to metadata table #8900

Uh oh!

Conversation

suryaprasanna commented Jun 7, 2023 • edited 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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danny0405 Jun 9, 2023 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codope left a comment

Choose a reason for hiding this comment

Uh oh!

codope commented Jun 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hudi-bot commented Jun 9, 2023

CI report:

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!

Reviewers

Assignees

Labels

suryaprasanna commented Jun 7, 2023 •

edited

Loading

danny0405 Jun 9, 2023 •

edited

Loading

codope commented Jun 9, 2023 •

edited

Loading