You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "TheR1sing3un (via GitHub)" <gi...@apache.org> on 2025/07/24 04:29:21 UTC

[PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

TheR1sing3un opened a new pull request, #13606:
URL: https://github.com/apache/hudi/pull/13606

   In most of our scenarios, we use the clean incremental scanning form to ensure the stability of clean because our table has a large number of partitions and the update frequency of the partitions is relatively low. If we do not use incremental scanning to clean, then each time we need to load the files of all partitions of the entire table.
   
   ### Change Logs
   
   1. Archival considers retaining the `the earliest retain instant` in the clean plan
   
   
   ### Impact
   
   Enhance the stability of cleaning service
   
   ### Risk level (write none, low medium or high below)
   
   low
   
   
   ### Documentation Update
   
   none
   
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Change Logs and Impact were stated clearly
   - [x] Adequate tests were added if applicable
   - [x] CI passed
   


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2229840388


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   I don't think the cleaning progress should blocking the archiving in general.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2227313529


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieArchivalConfig.java:
##########
@@ -102,6 +102,13 @@ public class HoodieArchivalConfig extends HoodieConfig {
       .withDocumentation("If enabled, archival will proceed beyond savepoint, skipping savepoint commits."
           + " If disabled, archival will stop at the earliest savepoint commit.");
 
+  public static final ConfigProperty<Boolean> ARCHIVE_KEEP_CLEAN_PLAN_RETAIN_INSTANT = ConfigProperty
+      .key("hoodie.archive.keep.clean.plan.retain.instant")
+      .defaultValue(false)

Review Comment:
   The default value is `false` to ensure that job hudi version upgrades do not affect the original logic of archive



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2229860901


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   > Or could we solve my core problem in another way: not wanting the incremental partition scanning in cleaning to always fall back to full table scanning. At present, the main reason for the fallback is that the `retain instant` was archived. Do you have any better ideas?
   
   In the case where retain instant is archival, do we still scan the incremental partition by reading the archive timeline and the active timeline? Is this plan feasible?
   



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on PR #13606:
URL: https://github.com/apache/hudi/pull/13606#issuecomment-3112751386

   @danny0405 Hi, Danny, Azure CI passed but Java CI failed. Is this a flaky test?


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228146279


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   it is resolved in `ClusteringUtils.getEarliestInstantToRetainForClustering`



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228162581


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   > it is resolved in `ClusteringUtils.getEarliestInstantToRetainForClustering`
   
   These two logics are not equivalent. In method `ClusteringUtils.getEarliestInstantToRetainForClustering`, it is only to find the first `clustering` instant that is larger than or equals to `retain instant` as a candidate. 
   <img width="1450" height="814" alt="image" src="https://github.com/user-attachments/assets/c723cffe-5597-45f9-86b6-e062f3efa895" />
   
   It is not equivalent to directly using `retain instant` as a candidate.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13606:
URL: https://github.com/apache/hudi/pull/13606#issuecomment-3111928975

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=7050",
       "triggerID" : "ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=7050) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13606:
URL: https://github.com/apache/hudi/pull/13606#issuecomment-3112527173

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=7050",
       "triggerID" : "ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=7050) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228224811


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   If the retain instant exists in the clean plan, there must be a replace commit on the timeline, this is by design.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228522065


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   > `retain in clean plan` is just a marker for left boundary of the incremental cleaning. It is not the constraint for archving for general, this variable is mainly introduced to resolve clustering data duplication issues as in the doc: "the clustering instant won't be archived before cleaned, and the earliest inflight clustering instant has a previous commit"
   
   Yes, you're right. All I want to do is to, on this basis, reserve a few more instances in active timeline to avoid the incremental partition scan fallback to a full table scan during cleaning. This does not seem to disrupt any of the original logic; it merely optimizes the performance of cleaning on the basis of compatibility with the original logic



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228175496


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   > It is not equivalent to directly using `retain instant` as a candidate.
   
   For example:
   
   ts_0: delta_commit
   ts_1: delta_commit
   ts_2: replace commit
   ts_3: clean, `retain instant`: ts_1
   
   now archive:
   
   `ClusteringUtils.getEarliestInstantToRetainForClustering` will return `ts_2` as archival retain candidate.
   
   But I want to regard `ts_1` as archival retain candidate, because if the archiver archived `ts_1`,  the next cleaning will fallback to full table scan.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228228292


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   > If the retain instant exists in the clean plan, there must be a replace commit on the timeline, this is by design.
   
   However, the `retain` instant in the clean plan is not necessarily the `replace commit` instant.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13606:
URL: https://github.com/apache/hudi/pull/13606#issuecomment-3111922159

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * ccc3151ff765a6bff3a162394d0d6cbac2ad3fb4 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on PR #13606:
URL: https://github.com/apache/hudi/pull/13606#issuecomment-3318731434

   @danny0405 Hi, Danny, are there any other suggestions? If this idea is not very good, I can close it for now, and let's reopen it when we have good ideas.


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228522065


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   > `retain in clean plan` is just a marker for left boundary of the incremental cleaning. It is not the constraint for archving for general, this variable is mainly introduced to resolve clustering data duplication issues as in the doc: "the clustering instant won't be archived before cleaned, and the earliest inflight clustering instant has a previous commit"
   
   Yes, you're right. All I want to do is to, on this basis, reserve a few more instants in active timeline to avoid the incremental partition scan fallback to a full table scan during cleaning. This does not seem to disrupt any of the original logic; it merely optimizes the performance of cleaning on the basis of compatibility with the original logic



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2229859367


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   > I don't think the cleaning progress should blocking the archiving in general.
   
   However, in fact, the current archival has been directly or indirectly affected by cleaning. For example, the number of commits that archival needs to retain needs to be adjusted according to the cleaning strategy, and the first replace commit after the `retain instant` retained by cleaning needs to be considered.
   
   Or could we solve my core problem in another way: not wanting the incremental partition scanning in cleaning to always fall back to full table scanning. At present, the main reason for the fallback is that the `retain instant` was archived. Do you have any better ideas? 



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228231968


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   > > It is not equivalent to directly using `retain instant` as a candidate.
   > 
   > For example:
   > 
   > ts_0: delta_commit ts_1: delta_commit ts_2: replace commit ts_3: clean, `retain instant`: ts_1
   > 
   > now archive:
   > 
   > `ClusteringUtils.getEarliestInstantToRetainForClustering` will return `ts_2` as archival retain candidate.
   > 
   > But I want to regard `ts_1` as archival retain candidate, because if the archiver archived `ts_1`, the next cleaning will fallback to full table scan.
   
   In this example, `replace commit` exists. The core issue is whether we consider retaining the archive to this `retain  in clean plan` rather than finding this  instant that is greater than or equal to this `retain in clean plan`



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [HUDI-9634] Archival considers retaining the `the earliest retain instant` in the clean plan [hudi]

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #13606:
URL: https://github.com/apache/hudi/pull/13606#discussion_r2228476012


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/versioning/v2/TimelineArchiverV2.java:
##########
@@ -256,6 +260,25 @@ private List<HoodieInstant> getCommitInstantsToArchive() throws IOException {
       earliestInstantToRetainCandidates.add(qualifiedEarliestInstant);
     }
 
+    // 6. If archival should consider `earliest retain instant` in the clean plan,
+    // we should add the earliest retain instant from the clean plan to the candidates.
+    if (config.shouldArchiveKeepCleanPlanRetainInstant()) {

Review Comment:
   `retain in clean plan` is just a marker for left boundary of the incremental cleaning. It is not the constraint for archving for general, this variable is mainly introduced to resolve clustering data duplication issues as in the doc:
   "the clustering instant won't be archived before cleaned, and the earliest inflight clustering instant has a previous commit"



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org