You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "suryaprasanna (via GitHub)" <gi...@apache.org> on 2023/05/22 21:55:48 UTC

[GitHub] [hudi] suryaprasanna opened a new pull request, #8783: Archival enhaceements

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

   ### Change Logs
   
   _Describe context and summary for this change. Highlight if any code was copied._
   
   ### Impact
   
   _Describe any public API or user-facing feature change or any performance impact._
   
   ### Risk level (write none, low medium or high below)
   
   _If medium or high, explain what verification was done to mitigate the risks._
   
   ### 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](https://hudi.apache.org/contribute/developer-setup#website) to make
     changes to the website._
   
   ### Contributor's checklist
   
   - [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [ ] Change Logs and Impact were stated clearly
   - [ ] Adequate tests were added if applicable
   - [ ] 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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431",
       "triggerID" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * 3b336af18e86b62c8c0c763d19669111d7d95122 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431) 
   
   <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


[GitHub] [hudi] nsivabalan commented on a diff in pull request #8783: Maintain commit timeline even in case of long standing inflights

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


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/HoodieTimelineArchiver.java:
##########
@@ -460,19 +465,12 @@ private Stream<HoodieInstant> getCommitInstantsToArchive() throws IOException {
               return !(firstSavepoint.isPresent() && compareTimestamps(firstSavepoint.get().getTimestamp(), LESSER_THAN_OR_EQUALS, s.getTimestamp()));
             }
           }).filter(s -> {
-            // Ensure commits >= the oldest pending compaction/replace commit is retained
-            return oldestPendingCompactionAndReplaceInstant
+            // oldestCommitToRetain is the highest completed commit instant that is less than the oldest inflight instant.
+            // By filter out any commit >= oldestCommitToRetain, we can ensure there are no gaps in the timeline
+            // when inflight commits are present.
+            return oldestCommitToRetain

Review Comment:
   this simplifies a lot :) 



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/io/TestHoodieTimelineArchiver.java:
##########
@@ -1416,6 +1418,91 @@ public void testArchivalWithMaxDeltaCommitsGuaranteeForCompaction(boolean enable
     }
   }
 
+  @Test

Review Comment:
   can we add some java docs on what we are testing here. 
   simple illustration of timeline might also help



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/io/TestHoodieTimelineArchiver.java:
##########
@@ -1416,6 +1418,91 @@ public void testArchivalWithMaxDeltaCommitsGuaranteeForCompaction(boolean enable
     }
   }
 
+  @Test
+  public void testGetCommitInstantsToArchiveDuringInflightCommits() throws Exception {
+    HoodieWriteConfig cfg = initTestTableAndGetWriteConfig(true, 4, 5, 5);
+    HoodieTestDataGenerator.createCommitFile(basePath, "100", wrapperFs.getConf());
+
+    HoodieTestDataGenerator.createReplaceCommitRequestedFile(basePath, "101", wrapperFs.getConf());
+    HoodieTestDataGenerator.createReplaceCommitInflightFile(basePath, "101", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "102", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "103", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCompactionRequestedFile(basePath, "104", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCompactionAuxiliaryMetadata(basePath,
+        new HoodieInstant(State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, "104"), wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "105", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "106", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "107", wrapperFs.getConf());
+    HoodieTable table = HoodieSparkTable.create(cfg, context, metaClient);
+    HoodieTimelineArchiver archiver = new HoodieTimelineArchiver(cfg, table);
+
+    HoodieTimeline timeline = metaClient.getActiveTimeline().getWriteTimeline();//getCommitsAndCompactionTimeline();
+    assertEquals(8, timeline.countInstants(), "Loaded 8 commits and the count should match");
+    boolean result = archiver.archiveIfRequired(context);
+    assertTrue(result);
+    timeline = metaClient.getActiveTimeline().reload().getWriteTimeline();//getCommitsAndCompactionTimeline();
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "100")),
+        "At least one instant before oldest pending replacecommit need to stay in the timeline");
+    assertEquals(8, timeline.countInstants(),
+        "Since we have a pending replacecommit at 101, we should never archive any commit "
+            + "after 101 and also to maintain timeline have at least one completed commit before pending commit");
+    assertTrue(timeline.containsInstant(new HoodieInstant(State.INFLIGHT, HoodieTimeline.REPLACE_COMMIT_ACTION, "101")),
+        "Inflight replacecommit must still be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "102")),
+        "Instants greater than oldest pending commit must be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "103")),
+        "Instants greater than oldest pending commit must be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, "104")),
+        "Instants greater than oldest pending commit must be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "105")),
+        "Instants greater than oldest pending commit must be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "106")),
+        "Instants greater than oldest pending commit must be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "107")),
+        "Instants greater than oldest pending commit must be present");
+  }
+
+  /**
+   * If replacecommit inflight is the oldest commit in the timeline
+   * then this method tests the archival logic to work by ignoring the inflight commit.
+   */
+  @Test
+  public void testWithOldestReplaceCommit() throws Exception {
+    HoodieWriteConfig cfg = initTestTableAndGetWriteConfig(false, 2, 3, 2);
+
+    HoodieTestDataGenerator.createReplaceCommitRequestedFile(basePath, "101", wrapperFs.getConf());
+    HoodieTestDataGenerator.createReplaceCommitInflightFile(basePath, "101", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "102", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCompactionRequestedFile(basePath, "103", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCompactionAuxiliaryMetadata(basePath,
+        new HoodieInstant(State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, "103"), wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "104", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "105", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "106", wrapperFs.getConf());
+    HoodieTable table = HoodieSparkTable.create(cfg, context, metaClient);
+    HoodieTimelineArchiver archiver = new HoodieTimelineArchiver(cfg, table);
+
+    HoodieTimeline timeline = metaClient.getActiveTimeline().getWriteTimeline();//getCommitsAndCompactionTimeline();
+    assertEquals(6, timeline.countInstants(), "Loaded 6 commits and the count should match");
+    boolean result = archiver.archiveIfRequired(context);
+    assertTrue(result);
+    timeline = metaClient.getActiveTimeline().reload().getWriteTimeline();//getCommitsAndCompactionTimeline();
+    assertEquals(6, timeline.countInstants(),
+        "Since we have a pending compaction at 101, we should never archive any commit after 101");
+    assertTrue(timeline.containsInstant(new HoodieInstant(State.INFLIGHT, HoodieTimeline.REPLACE_COMMIT_ACTION, "101")),
+        "Iiflight replacecommit must still be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "102")),
+        "Instants greater than oldest pending commit must be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, "103")),
+        "Instants greater than oldest pending commit must be present");

Review Comment:
   same here



##########
hudi-common/src/main/java/org/apache/hudi/common/util/ClusteringUtils.java:
##########
@@ -256,7 +256,12 @@ public static Option<HoodieInstant> getOldestInstantToRetainForClustering(
           retainLowerBound = earliestInstantToRetain.getTimestamp();
         } else {
           // no earliestInstantToRetain, indicate KEEP_LATEST_FILE_VERSIONS clean policy,
-          // retain first instant after clean instant
+          // retain first instant after clean instant.
+          // For KEEP_LATEST_FILE_VERSIONS cleaner policy, file versions are only maintained for active file groups
+          // not for replaced file groups. So, last clean instant can be considered as a lower bound, since
+          // the cleaner would have removed all the file groups until then. But there is a catch to this logic,
+          // while cleaner is running if there is a pending replacecommit then those files are not cleaned.
+          // TODO: This case has to be handled.

Review Comment:
   can you file a tracking ticket for this and link here



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/io/TestHoodieTimelineArchiver.java:
##########
@@ -1416,6 +1418,91 @@ public void testArchivalWithMaxDeltaCommitsGuaranteeForCompaction(boolean enable
     }
   }
 
+  @Test
+  public void testGetCommitInstantsToArchiveDuringInflightCommits() throws Exception {
+    HoodieWriteConfig cfg = initTestTableAndGetWriteConfig(true, 4, 5, 5);
+    HoodieTestDataGenerator.createCommitFile(basePath, "100", wrapperFs.getConf());
+
+    HoodieTestDataGenerator.createReplaceCommitRequestedFile(basePath, "101", wrapperFs.getConf());
+    HoodieTestDataGenerator.createReplaceCommitInflightFile(basePath, "101", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "102", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "103", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCompactionRequestedFile(basePath, "104", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCompactionAuxiliaryMetadata(basePath,
+        new HoodieInstant(State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, "104"), wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "105", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "106", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "107", wrapperFs.getConf());
+    HoodieTable table = HoodieSparkTable.create(cfg, context, metaClient);
+    HoodieTimelineArchiver archiver = new HoodieTimelineArchiver(cfg, table);
+
+    HoodieTimeline timeline = metaClient.getActiveTimeline().getWriteTimeline();//getCommitsAndCompactionTimeline();
+    assertEquals(8, timeline.countInstants(), "Loaded 8 commits and the count should match");
+    boolean result = archiver.archiveIfRequired(context);
+    assertTrue(result);
+    timeline = metaClient.getActiveTimeline().reload().getWriteTimeline();//getCommitsAndCompactionTimeline();
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "100")),
+        "At least one instant before oldest pending replacecommit need to stay in the timeline");
+    assertEquals(8, timeline.countInstants(),
+        "Since we have a pending replacecommit at 101, we should never archive any commit "
+            + "after 101 and also to maintain timeline have at least one completed commit before pending commit");
+    assertTrue(timeline.containsInstant(new HoodieInstant(State.INFLIGHT, HoodieTimeline.REPLACE_COMMIT_ACTION, "101")),
+        "Inflight replacecommit must still be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "102")),

Review Comment:
   can you declare a list of instants to expect in active timeline. and run it in a for loop rather than 1 line for each 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


[GitHub] [hudi] danny0405 commented on pull request #8783: [HUDI-5569] Maintain commit timeline even in case of long standing inflights

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

   @suryaprasanna Hi, can you update and fix the test failures?


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


[GitHub] [hudi] suryaprasanna commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   @danny0405 Thanks for reviewing this PR. Added more context to the PR. Please take a look.


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


[GitHub] [hudi] hudi-bot commented on pull request #8783: [HUDI-5569] Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431",
       "triggerID" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "triggerType" : "PUSH"
     }, {
       "hash" : "da86a92581b635744d4e844a8cdbec1a58f6a8ec",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17734",
       "triggerID" : "da86a92581b635744d4e844a8cdbec1a58f6a8ec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * 3b336af18e86b62c8c0c763d19669111d7d95122 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431) 
   * da86a92581b635744d4e844a8cdbec1a58f6a8ec Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17734) 
   
   <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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 9dbf3aa2367d1d78221a90ee1555188838424909 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280) 
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * 6fbff7384301b6e271c98c6ebcea24d838b52dd0 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392) 
   
   <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


[GitHub] [hudi] danny0405 commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   > In the past we have seen issues, when oldest inflight commit is the first commit in the timeline. In that case readers treated the files written by the oldest files as valid
   
   I'm wondering what instant the reader is holding to query the table, we have a check inside the fs view, only fileslice with the complete instant can be explosed to readers.


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


[GitHub] [hudi] danny0405 commented on a diff in pull request #8783: [HUDI-5569] Maintain commit timeline even in case of long standing inflights

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


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/HoodieTimelineArchiver.java:
##########
@@ -460,19 +465,12 @@ private Stream<HoodieInstant> getCommitInstantsToArchive() throws IOException {
               return !(firstSavepoint.isPresent() && compareTimestamps(firstSavepoint.get().getTimestamp(), LESSER_THAN_OR_EQUALS, s.getTimestamp()));
             }
           }).filter(s -> {
-            // Ensure commits >= the oldest pending compaction/replace commit is retained
-            return oldestPendingCompactionAndReplaceInstant
+            // oldestCommitToRetain is the highest completed commit instant that is less than the oldest inflight instant.
+            // By filter out any commit >= oldestCommitToRetain, we can ensure there are no gaps in the timeline
+            // when inflight commits are present.
+            return oldestCommitToRetain

Review Comment:
   Yeah, it can fixes https://github.com/apache/hudi/pull/7738, although a little obscure to understand.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/HoodieTimelineArchiver.java:
##########
@@ -460,19 +465,12 @@ private Stream<HoodieInstant> getCommitInstantsToArchive() throws IOException {
               return !(firstSavepoint.isPresent() && compareTimestamps(firstSavepoint.get().getTimestamp(), LESSER_THAN_OR_EQUALS, s.getTimestamp()));
             }
           }).filter(s -> {
-            // Ensure commits >= the oldest pending compaction/replace commit is retained
-            return oldestPendingCompactionAndReplaceInstant
+            // oldestCommitToRetain is the highest completed commit instant that is less than the oldest inflight instant.
+            // By filter out any commit >= oldestCommitToRetain, we can ensure there are no gaps in the timeline
+            // when inflight commits are present.
+            return oldestCommitToRetain

Review Comment:
   Yeah, it can fix https://github.com/apache/hudi/pull/7738, although a little obscure to understand.



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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431",
       "triggerID" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * 6fbff7384301b6e271c98c6ebcea24d838b52dd0 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392) 
   * 3b336af18e86b62c8c0c763d19669111d7d95122 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431) 
   
   <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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * 6fbff7384301b6e271c98c6ebcea24d838b52dd0 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392) 
   * 3b336af18e86b62c8c0c763d19669111d7d95122 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


[GitHub] [hudi] suryaprasanna commented on pull request #8783: [HUDI-5569] Maintain commit timeline even in case of long standing inflights

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

   @danny0405 My bad, noticed that a fix went in recently(Jan 2023) which will block the issue from happening.


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


[GitHub] [hudi] danny0405 commented on pull request #8783: Archival enhancements

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

   @suryaprasanna Thanks for the contribution ~, can you elaborate a little more what are we want to improve for archiving?


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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Archival enhancements

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 9dbf3aa2367d1d78221a90ee1555188838424909 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280) 
   
   <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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Archival enhancements

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 9dbf3aa2367d1d78221a90ee1555188838424909 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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Archival enhancements

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 9dbf3aa2367d1d78221a90ee1555188838424909 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280) 
   
   <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


[GitHub] [hudi] danny0405 commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   >  when cleaner's earliestToRetain can cross the inflight instant 
   
   This could not happen.
   
   


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


[GitHub] [hudi] suryaprasanna commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   The bug where reader's are reading from inflight instant is resolved. 
   
   With this change, I am trying to consolidate the approach of handling inflight commits. Since, we are handling pending compaction and replacecommits separately and it could lead to various corner cases.
   For example, when cleaner's earliestToRetain can cross the inflight instant and when this inflight file is committed, we will have different versions of the files left in the partitions. These kind of various cases can play out.
   So, I was thinking we need to have similar logic for both archival and cleaner. So, if there is a inflight commit present then both the archival and cleaner cannot cross the largestCompletedInstant i.e. < oldestInflighiInstant.
   We can say cleaner's earliestCommitToRetain similarly archival's max instant to archive cannot cross this commit.
   
   That is why I am trying to keep largestCompletedInstant that is less than the oldestInflightinstant from getting archived.


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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 9dbf3aa2367d1d78221a90ee1555188838424909 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280) 
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * 6fbff7384301b6e271c98c6ebcea24d838b52dd0 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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 9dbf3aa2367d1d78221a90ee1555188838424909 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280) 
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 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


[GitHub] [hudi] hudi-bot commented on pull request #8783: Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * 6fbff7384301b6e271c98c6ebcea24d838b52dd0 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392) 
   
   <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


[GitHub] [hudi] suryaprasanna commented on a diff in pull request #8783: [HUDI-5569] Maintain commit timeline even in case of long standing inflights

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


##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/io/TestHoodieTimelineArchiver.java:
##########
@@ -1416,6 +1418,91 @@ public void testArchivalWithMaxDeltaCommitsGuaranteeForCompaction(boolean enable
     }
   }
 
+  @Test

Review Comment:
   Added.



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/io/TestHoodieTimelineArchiver.java:
##########
@@ -1416,6 +1418,91 @@ public void testArchivalWithMaxDeltaCommitsGuaranteeForCompaction(boolean enable
     }
   }
 
+  @Test
+  public void testGetCommitInstantsToArchiveDuringInflightCommits() throws Exception {
+    HoodieWriteConfig cfg = initTestTableAndGetWriteConfig(true, 4, 5, 5);
+    HoodieTestDataGenerator.createCommitFile(basePath, "100", wrapperFs.getConf());
+
+    HoodieTestDataGenerator.createReplaceCommitRequestedFile(basePath, "101", wrapperFs.getConf());
+    HoodieTestDataGenerator.createReplaceCommitInflightFile(basePath, "101", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "102", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "103", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCompactionRequestedFile(basePath, "104", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCompactionAuxiliaryMetadata(basePath,
+        new HoodieInstant(State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, "104"), wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "105", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "106", wrapperFs.getConf());
+    HoodieTestDataGenerator.createCommitFile(basePath, "107", wrapperFs.getConf());
+    HoodieTable table = HoodieSparkTable.create(cfg, context, metaClient);
+    HoodieTimelineArchiver archiver = new HoodieTimelineArchiver(cfg, table);
+
+    HoodieTimeline timeline = metaClient.getActiveTimeline().getWriteTimeline();//getCommitsAndCompactionTimeline();
+    assertEquals(8, timeline.countInstants(), "Loaded 8 commits and the count should match");
+    boolean result = archiver.archiveIfRequired(context);
+    assertTrue(result);
+    timeline = metaClient.getActiveTimeline().reload().getWriteTimeline();//getCommitsAndCompactionTimeline();
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "100")),
+        "At least one instant before oldest pending replacecommit need to stay in the timeline");
+    assertEquals(8, timeline.countInstants(),
+        "Since we have a pending replacecommit at 101, we should never archive any commit "
+            + "after 101 and also to maintain timeline have at least one completed commit before pending commit");
+    assertTrue(timeline.containsInstant(new HoodieInstant(State.INFLIGHT, HoodieTimeline.REPLACE_COMMIT_ACTION, "101")),
+        "Inflight replacecommit must still be present");
+    assertTrue(timeline.containsInstant(new HoodieInstant(false, HoodieTimeline.COMMIT_ACTION, "102")),

Review Comment:
   Sure, refactored the code.



##########
hudi-common/src/main/java/org/apache/hudi/common/util/ClusteringUtils.java:
##########
@@ -256,7 +256,12 @@ public static Option<HoodieInstant> getOldestInstantToRetainForClustering(
           retainLowerBound = earliestInstantToRetain.getTimestamp();
         } else {
           // no earliestInstantToRetain, indicate KEEP_LATEST_FILE_VERSIONS clean policy,
-          // retain first instant after clean instant
+          // retain first instant after clean instant.
+          // For KEEP_LATEST_FILE_VERSIONS cleaner policy, file versions are only maintained for active file groups
+          // not for replaced file groups. So, last clean instant can be considered as a lower bound, since
+          // the cleaner would have removed all the file groups until then. But there is a catch to this logic,
+          // while cleaner is running if there is a pending replacecommit then those files are not cleaned.
+          // TODO: This case has to be handled.

Review Comment:
   Created couple of tickets please review them.
   https://issues.apache.org/jira/browse/HUDI-6351
   https://issues.apache.org/jira/browse/HUDI-6352
   



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


[GitHub] [hudi] hudi-bot commented on pull request #8783: [HUDI-5569] Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431",
       "triggerID" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "triggerType" : "PUSH"
     }, {
       "hash" : "da86a92581b635744d4e844a8cdbec1a58f6a8ec",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "da86a92581b635744d4e844a8cdbec1a58f6a8ec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * 3b336af18e86b62c8c0c763d19669111d7d95122 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431) 
   * da86a92581b635744d4e844a8cdbec1a58f6a8ec 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


[GitHub] [hudi] hudi-bot commented on pull request #8783: [HUDI-5569] Maintain commit timeline even in case of long standing inflights

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17280",
       "triggerID" : "9dbf3aa2367d1d78221a90ee1555188838424909",
       "triggerType" : "PUSH"
     }, {
       "hash" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "0cce98bd4802329b8d4b414b1d0ef85128e29a53",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17392",
       "triggerID" : "6fbff7384301b6e271c98c6ebcea24d838b52dd0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17431",
       "triggerID" : "3b336af18e86b62c8c0c763d19669111d7d95122",
       "triggerType" : "PUSH"
     }, {
       "hash" : "da86a92581b635744d4e844a8cdbec1a58f6a8ec",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17734",
       "triggerID" : "da86a92581b635744d4e844a8cdbec1a58f6a8ec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 0cce98bd4802329b8d4b414b1d0ef85128e29a53 UNKNOWN
   * da86a92581b635744d4e844a8cdbec1a58f6a8ec Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17734) 
   
   <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


[GitHub] [hudi] nsivabalan merged pull request #8783: [HUDI-5569] Maintain commit timeline even in case of long standing inflights

Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan merged PR #8783:
URL: https://github.com/apache/hudi/pull/8783


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