You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "danny0405 (via GitHub)" <gi...@apache.org> on 2026/08/06 02:02:59 UTC

[PR] test: improve upgrade and async index coverage [hudi]

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

   ### Describe the issue this Pull Request addresses
   
   Several table upgrade/downgrade handlers, async-index actions, and bucket-index utilities in `hudi-client-common` had significant line-coverage gaps. Critical paths such as legacy table-version transitions, index catch-up with concurrent commits, abort cleanup, bucket assignment, and consistent-hashing metadata recovery were not exercised directly.
   
   This PR adds focused unit and fixture-backed coverage for those paths without changing production code, public APIs, configuration, or storage formats.
   
   ### Summary and Changelog
   
   - Add legacy handler tests for `ZeroToOneUpgradeHandler`, `TwoToOneDowngradeHandler`, `FourToFiveUpgradeHandler`, and `FiveToSixUpgradeHandler`, including config and file-layout transitions and failure paths.
   - Extend `TestUpgradeDowngrade` with fixture-backed v4-to-v5 and v5-to-v6 upgrades and verify data consistency after each transition.
   - Exercise `UpgradeDowngrade` version-hop orchestration, rollback/compaction selection, property changes, no-op validation, and failure handling.
   - Exercise async index scheduling, execution, concurrent-commit catch-up, metadata partition initialization, and partial-index cleanup.
   - Add direct coverage for simple bucket assignment/lookup and consistent-hashing metadata load, save, marker repair, and concurrent-creation recovery.
   - No code was copied.
   
   Per-class line coverage:
   
   | Class | Before (Codecov) | After (focused JaCoCo) |
   | --- | ---: | ---: |
   | `ZeroToOneUpgradeHandler` | 0% | 87.8% |
   | `TwoToOneDowngradeHandler` | 0% | 88.6% |
   | `FourToFiveUpgradeHandler` | 0% | 100.0% |
   | `FiveToSixUpgradeHandler` | 0% | 100.0% |
   | `UpgradeDowngrade` | 76% | 75.1%* |
   | `AbstractIndexingCatchupTask` | 42% | 88.4% |
   | `RunIndexActionExecutor` | 72% | 94.6% |
   | `ScheduleIndexActionExecutor` | 69% | 86.4% |
   | `HoodieSimpleBucketIndex` | 46% | 100.0% |
   | `ConsistentBucketIndexUtils` | 76% | 78.8% |
   
   \* The focused report excludes pre-existing suites; merged Codecov coverage retains their existing contribution.
   
   Validation:
   
   - `mvn -pl hudi-client/hudi-client-common -Punit-tests -Drat.skip=true -Dtest=TestLegacyUpgradeDowngradeHandlers,TestIndexingCatchupTask,TestIndexActionExecutors,TestHoodieSimpleBucketIndex,TestConsistentBucketIndexUtils,TestUpgradeDowngradeOrchestration test` — 46 tests passed.
   - `mvn -pl hudi-spark-datasource/hudi-spark -Dcheckstyle.skip -Drat.skip=true -Djacoco.skip=true -Dtest=TestUpgradeDowngrade#testLegacyUpgradeHandlersWithFixtureTables surefire:test` — 2 tests passed.
   - `mvn -pl hudi-client/hudi-client-common checkstyle:check` — 0 violations.
   - `mvn -pl hudi-spark-datasource/hudi-spark checkstyle:check` — 0 violations.
   - `git diff --check` — passed.
   
   ### Impact
   
   Test-only change. There is no public API, configuration, storage-format, user-facing behavior, or performance impact.
   
   ### Risk Level
   
   low
   
   The change only adds and extends tests. Focused unit and Spark fixture tests passed, and both affected modules pass Checkstyle.
   
   ### Documentation Update
   
   none — this PR does not add or change features, configuration, APIs, or user-facing behavior.
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable
   


-- 
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] test: improve upgrade and async index coverage [hudi]

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115",
       "triggerID" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * a47277ecf15f2b2c2d4f549417a190208a302194 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115) 
   
   <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] fix: harden async indexing and improve coverage [hudi]

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


-- 
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] test: improve upgrade and async index coverage [hudi]

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115",
       "triggerID" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d7887698fe768780d81ae6026332e006678dbb0d",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16156",
       "triggerID" : "d7887698fe768780d81ae6026332e006678dbb0d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * d7887698fe768780d81ae6026332e006678dbb0d Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16156) 
   
   <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] test: improve upgrade and async index coverage [hudi]

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


##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestLegacyUpgradeDowngradeHandlers.java:
##########
@@ -0,0 +1,352 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.table.upgrade;
+
+import org.apache.hudi.common.HoodieRollbackStat;
+import org.apache.hudi.common.engine.HoodieEngineContext;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.marker.MarkerType;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.InstantFileNameGenerator;
+import org.apache.hudi.common.util.HoodieStorageUtils;
+import org.apache.hudi.common.util.MarkerUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.HoodieRollbackException;
+import org.apache.hudi.exception.HoodieUpgradeDowngradeException;
+import org.apache.hudi.storage.HoodieStorage;
+import org.apache.hudi.storage.StoragePath;
+import org.apache.hudi.storage.StoragePathInfo;
+import org.apache.hudi.table.HoodieTable;
+import org.apache.hudi.table.marker.DirectWriteMarkers;
+import org.apache.hudi.table.marker.WriteMarkers;
+import org.apache.hudi.table.marker.WriteMarkersFactory;
+
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static org.apache.hudi.common.testutils.HoodieTestUtils.INSTANT_GENERATOR;
+import static org.apache.hudi.common.testutils.HoodieTestUtils.getDefaultStorageConf;
+import static org.apache.hudi.common.util.PartitionPathEncodeUtils.DEPRECATED_DEFAULT_PARTITION_PATH;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockConstruction;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+class TestLegacyUpgradeDowngradeHandlers {
+
+  @Test
+  void testZeroToOneRecreatesMarkersAndSkipsCurrentInstant() {
+    HoodieTable table = mockTableWithPendingInstants("001", "002");
+    HoodieEngineContext context = mock(HoodieEngineContext.class);
+    doReturn(getDefaultStorageConf()).when(context).getStorageConf();
+    HoodieWriteConfig config = mock(HoodieWriteConfig.class);
+    SupportsUpgradeDowngrade helper = mock(SupportsUpgradeDowngrade.class);
+    when(helper.getTable(config, context)).thenReturn(table);
+    when(config.getMarkersDeleteParallelism()).thenReturn(3);
+
+    ZeroToOneUpgradeHandler handler = org.mockito.Mockito.spy(new ZeroToOneUpgradeHandler());
+    org.mockito.Mockito.doNothing().when(handler).recreateMarkers(anyString(), eq(table), eq(context), anyInt());

Review Comment:
   🤖 nit: `org.mockito.Mockito.spy` and `org.mockito.Mockito.doNothing()` are used fully-qualified on these two lines while the file already statically imports `mock`, `verify`, `doReturn`, etc. — could you add `spy` and `doNothing` to the static imports for consistency?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>



-- 
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] test: improve upgrade and async index coverage [hudi]

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115",
       "triggerID" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d7887698fe768780d81ae6026332e006678dbb0d",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "d7887698fe768780d81ae6026332e006678dbb0d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * a47277ecf15f2b2c2d4f549417a190208a302194 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115) 
   * d7887698fe768780d81ae6026332e006678dbb0d 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] test: improve upgrade and async index coverage [hudi]

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * a47277ecf15f2b2c2d4f549417a190208a302194 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] test: improve upgrade and async index coverage [hudi]

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115",
       "triggerID" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d7887698fe768780d81ae6026332e006678dbb0d",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16156",
       "triggerID" : "d7887698fe768780d81ae6026332e006678dbb0d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * a47277ecf15f2b2c2d4f549417a190208a302194 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115) 
   * d7887698fe768780d81ae6026332e006678dbb0d Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16156) 
   
   <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] test: improve upgrade and async index coverage [hudi]

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


##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/action/index/TestIndexingCatchupTask.java:
##########
@@ -240,6 +249,121 @@ public void testNoHeartbeat() throws IOException {
     assertTrue(task.awaitInstantCaughtUp(pendingInstantWithNoHeartbeat), "Expected null as the instant's heartbeat has expired.");
   }
 
+  @Test
+  public void testRunSkipsInstantAlreadyCommittedToMetadata() {
+    HoodieInstant instant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.COMMIT_ACTION, "002");
+    HoodieActiveTimeline metadataTimeline = mock(HoodieActiveTimeline.class);
+    when(metadataMetaClient.reloadActiveTimeline()).thenReturn(metadataTimeline);
+    when(metadataTimeline.filterCompletedInstants()).thenReturn(metadataTimeline);
+    when(metadataTimeline.filter(any())).thenReturn(metadataTimeline);
+    when(metadataTimeline.firstInstant()).thenReturn(Option.of(instant));
+
+    RunningIndexingCatchupTask task = runningTask(instant);
+    task.run();
+
+    assertEquals("002", task.currentCaughtupInstant);
+    assertEquals(0, task.writeActionsUpdated.get());
+  }
+
+  @Test
+  public void testRunUpdatesCompletedWriteActionInsideTransaction() {
+    HoodieInstant instant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.REPLACE_COMMIT_ACTION, "002");
+    stubNoCompletedMetadataInstant();
+    RunningIndexingCatchupTask task = runningTask(instant);
+
+    task.run();
+
+    assertEquals(1, task.writeActionsUpdated.get());
+    assertEquals("002", task.currentCaughtupInstant);
+    verify(transactionManager).beginStateChange(Option.of(instant), Option.empty());
+    verify(transactionManager).endStateChange(Option.of(instant));
+  }
+
+  @Test
+  public void testRunUpdatesCleanRestoreAndRollbackActions() throws IOException {
+    HoodieInstant clean = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.CLEAN_ACTION, "002");
+    HoodieInstant restore = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.RESTORE_ACTION, "003");
+    HoodieInstant rollback = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.ROLLBACK_ACTION, "004");
+    stubNoCompletedMetadataInstant();
+    HoodieCleanMetadata cleanMetadata = mock(HoodieCleanMetadata.class);
+    HoodieRestoreMetadata restoreMetadata = mock(HoodieRestoreMetadata.class);
+    HoodieRollbackMetadata rollbackMetadata = mock(HoodieRollbackMetadata.class);
+    HoodieActiveTimeline activeTimeline = mock(HoodieActiveTimeline.class);
+    when(metaClient.getActiveTimeline()).thenReturn(activeTimeline);
+    when(activeTimeline.readRestoreMetadata(restore)).thenReturn(restoreMetadata);
+    when(activeTimeline.readRollbackMetadata(rollback)).thenReturn(rollbackMetadata);
+
+    try (MockedStatic<CleanerUtils> cleanerUtils = mockStatic(CleanerUtils.class)) {
+      cleanerUtils.when(() -> CleanerUtils.getCleanerMetadata(metaClient, clean)).thenReturn(cleanMetadata);
+      runningTask(clean, restore, rollback).run();
+    }
+
+    verify(metadataWriter).update(cleanMetadata, "002");
+    verify(metadataWriter).update(restoreMetadata, "003");
+    verify(metadataWriter).update(rollbackMetadata, "004");
+    verify(transactionManager).endStateChange(Option.of(clean));
+    verify(transactionManager).endStateChange(Option.of(restore));
+    verify(transactionManager).endStateChange(Option.of(rollback));
+  }
+
+  @Test
+  public void testRunRejectsUnexpectedCompletedActionAndReleasesTransaction() {
+    HoodieInstant instant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.SAVEPOINT_ACTION, "002");
+    stubNoCompletedMetadataInstant();
+
+    assertThrows(IllegalStateException.class, () -> runningTask(instant).run());
+    verify(transactionManager).endStateChange(Option.of(instant));
+  }
+
+  @Test
+  public void testAwaitInstantCaughtUpUsesKnownMetadataInstant() {
+    HoodieInstant instant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.INFLIGHT, HoodieTimeline.COMMIT_ACTION, "002");
+    RunningIndexingCatchupTask task = new RunningIndexingCatchupTask(
+        metadataWriter, Collections.singletonList(instant), Collections.singleton("002"), metaClient, metadataMetaClient,
+        transactionManager, "001", engineContext, table, heartbeatClient);
+
+    assertTrue(task.awaitInstantCaughtUp(instant));
+    assertEquals("002", task.currentCaughtupInstant);
+  }
+
+  private RunningIndexingCatchupTask runningTask(HoodieInstant... instants) {
+    return new RunningIndexingCatchupTask(
+        metadataWriter, java.util.Arrays.asList(instants), new HashSet<>(), metaClient, metadataMetaClient,
+        transactionManager, "001", engineContext, table, heartbeatClient);
+  }
+
+  private void stubNoCompletedMetadataInstant() {
+    HoodieActiveTimeline metadataTimeline = mock(HoodieActiveTimeline.class);
+    when(metadataMetaClient.reloadActiveTimeline()).thenReturn(metadataTimeline);
+    when(metadataTimeline.filterCompletedInstants()).thenReturn(metadataTimeline);
+    when(metadataTimeline.filter(any())).thenReturn(metadataTimeline);
+    when(metadataTimeline.firstInstant()).thenReturn(Option.empty());
+  }
+
+  static class RunningIndexingCatchupTask extends AbstractIndexingCatchupTask {
+    private final AtomicInteger writeActionsUpdated = new AtomicInteger();
+
+    RunningIndexingCatchupTask(HoodieTableMetadataWriter metadataWriter,
+                               List<HoodieInstant> instantsToIndex,
+                               Set<String> metadataCompletedInstants,
+                               HoodieTableMetaClient metaClient,
+                               HoodieTableMetaClient metadataMetaClient,
+                               TransactionManager transactionManager,
+                               String currentCaughtupInstant,
+                               HoodieEngineContext engineContext,
+                               HoodieTable table,
+                               HoodieHeartbeatClient heartbeatClient) {
+      super(metadataWriter, instantsToIndex, metadataCompletedInstants, metaClient, metadataMetaClient,
+          transactionManager, currentCaughtupInstant, engineContext, table, heartbeatClient);
+    }
+
+    @Override
+    public void updateIndexForWriteAction(HoodieInstant instant) {
+      writeActionsUpdated.incrementAndGet();
+      currentCaughtupInstant = instant.requestedTime();

Review Comment:
   **P1:** This test double advances `currentCaughtupInstant` itself, but neither production catch-up implementation does so. The executor test also supplies empty timelines and mocks `IndexingCatchupTask`, so it proves only that `run()` was called. In production, `RunIndexActionExecutor` passes the initial instant as a `String` and later builds `HoodieIndexCommitMetadata` from its unchanged field; after processing completed instant `002`, it still reports `001`. Please exercise a production catch-up task with a real completed instant, assert the returned `indexUptoInstant`, and propagate the last successfully processed instant back to the executor.



##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/action/index/TestIndexActionExecutors.java:
##########
@@ -0,0 +1,341 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.table.action.index;
+
+import org.apache.hudi.avro.model.HoodieIndexCommitMetadata;
+import org.apache.hudi.avro.model.HoodieIndexPartitionInfo;
+import org.apache.hudi.avro.model.HoodieIndexPlan;
+import org.apache.hudi.client.heartbeat.HoodieHeartbeatClient;
+import org.apache.hudi.client.transaction.TransactionManager;
+import org.apache.hudi.common.engine.HoodieEngineContext;
+import org.apache.hudi.common.model.WriteConcurrencyMode;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieArchivedTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieLockConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.core.transaction.lock.InProcessLockProvider;
+import org.apache.hudi.exception.HoodieIOException;
+import org.apache.hudi.exception.HoodieIndexException;
+import org.apache.hudi.metadata.HoodieTableMetadataUtil;
+import org.apache.hudi.metadata.HoodieTableMetadataWriter;
+import org.apache.hudi.metadata.MetadataPartitionType;
+import org.apache.hudi.storage.HoodieStorage;
+import org.apache.hudi.storage.StoragePath;
+import org.apache.hudi.table.HoodieTable;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.apache.hudi.common.testutils.HoodieTestUtils.INSTANT_GENERATOR;
+import static org.apache.hudi.common.testutils.HoodieTestUtils.getDefaultStorageConf;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockConstruction;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+class TestIndexActionExecutors {
+
+  private HoodieEngineContext context;
+  private HoodieTable table;
+  private HoodieTableMetaClient metaClient;
+  private HoodieTableConfig tableConfig;
+  private HoodieActiveTimeline activeTimeline;
+  private HoodieStorage storage;
+
+  @BeforeEach
+  void setUp() {
+    context = mock(HoodieEngineContext.class);
+    table = mock(HoodieTable.class);
+    metaClient = mock(HoodieTableMetaClient.class);
+    tableConfig = mock(HoodieTableConfig.class);
+    activeTimeline = mock(HoodieActiveTimeline.class);
+    storage = mock(HoodieStorage.class);
+    when(table.getMetaClient()).thenReturn(metaClient);
+    when(table.getActiveTimeline()).thenReturn(activeTimeline);
+    when(table.getInstantGenerator()).thenReturn(INSTANT_GENERATOR);
+    when(table.getStorage()).thenReturn(storage);
+    doReturn(getDefaultStorageConf()).when(storage).getConf();
+    doReturn(getDefaultStorageConf()).when(context).getStorageConf();
+    when(metaClient.getTableConfig()).thenReturn(tableConfig);
+    when(metaClient.getInstantGenerator()).thenReturn(INSTANT_GENERATOR);
+    when(metaClient.getActiveTimeline()).thenReturn(activeTimeline);
+    when(metaClient.reloadActiveTimeline()).thenReturn(activeTimeline);
+    when(tableConfig.getMetadataPartitions()).thenReturn(Collections.emptySet());
+    when(tableConfig.getMetadataPartitionsInflight()).thenReturn(Collections.emptySet());
+  }
+
+  @Test
+  void testScheduleRejectsSingleWriterConfiguration() {
+    HoodieWriteConfig config = HoodieWriteConfig.newBuilder().withPath("/table").build();
+    ScheduleIndexActionExecutor executor = new ScheduleIndexActionExecutor(
+        context, config, table, "002", Collections.singletonList(MetadataPartitionType.COLUMN_STATS), Collections.emptyList());
+
+    HoodieIndexException exception = assertThrows(HoodieIndexException.class, executor::execute);
+    assertTrue(exception.getMessage().contains(HoodieWriteConfig.WRITE_CONCURRENCY_MODE.key()));
+  }
+
+  @Test
+  void testScheduleCreatesPlanAndIsIdempotent() {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant completed = INSTANT_GENERATOR.createNewInstant(
+        HoodieInstant.State.COMPLETED, HoodieTimeline.COMMIT_ACTION, "001");
+    HoodieTimeline completedTimeline = mock(HoodieTimeline.class);
+    when(activeTimeline.getContiguousCompletedWriteTimeline()).thenReturn(completedTimeline);
+    when(completedTimeline.lastInstant()).thenReturn(Option.of(completed));
+    ScheduleIndexActionExecutor executor = new ScheduleIndexActionExecutor(
+        context, config, table, "002", Collections.singletonList(MetadataPartitionType.COLUMN_STATS), Collections.emptyList());
+
+    Option<HoodieIndexPlan> plan = executor.execute();
+
+    assertTrue(plan.isPresent());
+    assertEquals(1, plan.get().getIndexPartitionInfos().size());
+    assertEquals(MetadataPartitionType.COLUMN_STATS.getPartitionPath(),
+        plan.get().getIndexPartitionInfos().get(0).getMetadataPartitionPath());
+    assertEquals("001", plan.get().getIndexPartitionInfos().get(0).getIndexUptoInstant());
+    verify(activeTimeline).saveToPendingIndexAction(any(HoodieInstant.class), any(HoodieIndexPlan.class));
+
+    when(tableConfig.getMetadataPartitions()).thenReturn(Set.of(MetadataPartitionType.COLUMN_STATS.getPartitionPath()));
+    assertFalse(executor.execute().isPresent());
+  }
+
+  @Test
+  void testScheduleAbortsWhenPendingPlanCannotBeSaved() {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant completed = INSTANT_GENERATOR.createNewInstant(
+        HoodieInstant.State.COMPLETED, HoodieTimeline.COMMIT_ACTION, "001");
+    HoodieTimeline completedTimeline = mock(HoodieTimeline.class);
+    when(activeTimeline.getContiguousCompletedWriteTimeline()).thenReturn(completedTimeline);
+    when(completedTimeline.lastInstant()).thenReturn(Option.of(completed));
+    doThrow(new HoodieIOException("write failed"))
+        .when(activeTimeline).saveToPendingIndexAction(any(HoodieInstant.class), any(HoodieIndexPlan.class));
+
+    try (MockedStatic<HoodieTableMetadataUtil> metadataUtil = mockStatic(HoodieTableMetadataUtil.class)) {
+      metadataUtil.when(() -> HoodieTableMetadataUtil.getInflightAndCompletedMetadataPartitions(tableConfig))
+          .thenReturn(Collections.emptySet());
+      metadataUtil.when(() -> HoodieTableMetadataUtil.metadataPartitionExists(
+          metaClient.getBasePath(), context, MetadataPartitionType.COLUMN_STATS.getPartitionPath())).thenReturn(false);
+
+      Option<HoodieIndexPlan> result = new ScheduleIndexActionExecutor(
+          context, config, table, "002", Collections.singletonList(MetadataPartitionType.COLUMN_STATS), Collections.emptyList()).execute();
+
+      assertFalse(result.isPresent());
+      verify(activeTimeline).deleteInstantFileIfExists(INSTANT_GENERATOR.getIndexRequestedInstant("002"));
+    }
+  }
+
+  @Test
+  void testRunRejectsInvalidConfigurationAndMissingInstant() {
+    RunIndexActionExecutor invalidExecutor = new RunIndexActionExecutor(
+        context, HoodieWriteConfig.newBuilder().withPath("/table").build(), table, "002");
+    assertThrows(HoodieIndexException.class, invalidExecutor::execute);
+
+    HoodieWriteConfig config = multiWriterConfig();
+    when(activeTimeline.filterPendingIndexTimeline()).thenReturn(activeTimeline);
+    when(activeTimeline.filter(any())).thenReturn(activeTimeline);
+    when(activeTimeline.lastInstant()).thenReturn(Option.empty());
+    assertThrows(HoodieIndexException.class,
+        () -> new RunIndexActionExecutor(context, config, table, "002").execute());
+  }
+
+  @Test
+  void testRunRejectsUnreadableAndEmptyPlans() throws IOException {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant requested = requestedIndexInstant();
+    stubRequestedIndexInstant(requested);
+    doThrow(new IOException("read failed")).when(activeTimeline).readIndexPlan(requested);
+    assertThrows(HoodieIndexException.class,
+        () -> new RunIndexActionExecutor(context, config, table, "002").execute());
+
+    doReturn(new HoodieIndexPlan(1, Collections.emptyList())).when(activeTimeline).readIndexPlan(requested);
+    assertThrows(HoodieIndexException.class,
+        () -> new RunIndexActionExecutor(context, config, table, "002").execute());
+  }
+
+  @Test
+  void testRunRejectsPartitionThatAlreadyExists() throws IOException {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant requested = requestedIndexInstant();
+    stubRequestedIndexInstant(requested);
+    HoodieIndexPartitionInfo info = new HoodieIndexPartitionInfo(
+        1, MetadataPartitionType.COLUMN_STATS.getPartitionPath(), "001", Collections.emptyMap());
+    when(activeTimeline.readIndexPlan(requested)).thenReturn(new HoodieIndexPlan(1, Collections.singletonList(info)));
+    when(tableConfig.getMetadataPartitions()).thenReturn(Set.of(MetadataPartitionType.COLUMN_STATS.getPartitionPath()));
+
+    assertThrows(HoodieIndexException.class,
+        () -> new RunIndexActionExecutor(context, config, table, "002").execute());
+  }
+
+  @Test
+  void testRunInitializesFilesPartitionAndCompletesIndexInstant() throws IOException {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant requested = requestedIndexInstant();
+    stubRequestedIndexInstant(requested);
+    HoodieIndexPartitionInfo info = new HoodieIndexPartitionInfo(
+        1, MetadataPartitionType.FILES.getPartitionPath(), "001", Collections.emptyMap());
+    when(activeTimeline.readIndexPlan(requested)).thenReturn(new HoodieIndexPlan(1, Collections.singletonList(info)));
+    HoodieTableMetadataWriter writer = mock(HoodieTableMetadataWriter.class);
+    when(table.getIndexingMetadataWriter("002")).thenReturn(Option.of(writer));

Review Comment:
   **P2:** This mock represents an `AutoCloseable` metadata writer, but the FILES initialization branch obtains the writer and discards it without closing it. Unlike the non-FILES branch, there is no try-with-resources; the real writer's `close()` releases its metadata reader, write client, and view. A focused `verify(writer).close()` assertion fails. Please close the writer in the production FILES branch and assert closure on both successful initialization and timeline-completion failure.



-- 
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] fix: harden async indexing and improve coverage [hudi]

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


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/RunIndexActionExecutor.java:
##########
@@ -180,9 +180,13 @@ public Option<HoodieIndexCommitMetadata> execute() {
         String indexUptoInstant = fileIndexPartitionInfo.getIndexUptoInstant();
         // save index commit metadata and update table config
         // instantiation of metadata writer will automatically instantiate the partitions.
-        table.getIndexingMetadataWriter(instantTime)
+        try (HoodieTableMetadataWriter metadataWriter = table.getIndexingMetadataWriter(instantTime)

Review Comment:
   🤖 nit: an empty try-with-resources body that relies on the writer's construction side-effect reads as unusual. Could you make the intent more obvious, e.g. assign to a clearly-named local (`initializingWriter`) or keep the explanatory comment but consider whether an explicit `.get()`/no-op would read better?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>



-- 
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] test: improve upgrade and async index coverage [hudi]

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

   ## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/19537?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 76.40%. Comparing base ([`4a9cef8`](https://app.codecov.io/gh/apache/hudi/commit/4a9cef87c2c5c9b12160b17aea2340cc4e899bb1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a47277e`](https://app.codecov.io/gh/apache/hudi/commit/a47277ecf15f2b2c2d4f549417a190208a302194?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #19537      +/-   ##
   ============================================
   - Coverage     76.40%   76.40%   -0.01%     
     Complexity    32398    32398              
   ============================================
     Files          2520     2520              
     Lines        138985   138985              
     Branches      16695    16695              
   ============================================
   - Hits         106189   106187       -2     
   - Misses        25166    25167       +1     
   - Partials       7630     7631       +1     
   ```
   
   | [Components](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `82.35% <ø> (ø)` | |
   | [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.98% <ø> (-0.01%)` | :arrow_down: |
   | [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.97% <ø> (ø)` | |
   | [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.62% <ø> (ø)` | |
   | [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.67% <ø> (ø)` | |
   | [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.32% <ø> (ø)` | |
   | [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.50% <ø> (ø)` | |
   | [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.00% <ø> (ø)` | |
   | [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.46% <ø> (ø)` | |
   | [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.44% <ø> (ø)` | |
   | [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.06% <ø> (ø)` | |
   | [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19537/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.20% <ø> (ø)` | |
   
   | [Flag](https://app.codecov.io/gh/apache/hudi/pull/19537/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/19537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.76% <ø> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   [see 1 file with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/19537/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   </details>
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </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] test: improve upgrade and async index coverage [hudi]

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115",
       "triggerID" : "a47277ecf15f2b2c2d4f549417a190208a302194",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * a47277ecf15f2b2c2d4f549417a190208a302194 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16115) 
   
   <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] fix: harden async indexing and improve coverage [hudi]

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


##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestLegacyUpgradeDowngradeHandlers.java:
##########
@@ -0,0 +1,352 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.table.upgrade;
+
+import org.apache.hudi.common.HoodieRollbackStat;
+import org.apache.hudi.common.engine.HoodieEngineContext;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.marker.MarkerType;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.InstantFileNameGenerator;
+import org.apache.hudi.common.util.HoodieStorageUtils;
+import org.apache.hudi.common.util.MarkerUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.HoodieRollbackException;
+import org.apache.hudi.exception.HoodieUpgradeDowngradeException;
+import org.apache.hudi.storage.HoodieStorage;
+import org.apache.hudi.storage.StoragePath;
+import org.apache.hudi.storage.StoragePathInfo;
+import org.apache.hudi.table.HoodieTable;
+import org.apache.hudi.table.marker.DirectWriteMarkers;
+import org.apache.hudi.table.marker.WriteMarkers;
+import org.apache.hudi.table.marker.WriteMarkersFactory;
+
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static org.apache.hudi.common.testutils.HoodieTestUtils.INSTANT_GENERATOR;
+import static org.apache.hudi.common.testutils.HoodieTestUtils.getDefaultStorageConf;
+import static org.apache.hudi.common.util.PartitionPathEncodeUtils.DEPRECATED_DEFAULT_PARTITION_PATH;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockConstruction;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+class TestLegacyUpgradeDowngradeHandlers {
+
+  @Test
+  void testZeroToOneRecreatesMarkersAndSkipsCurrentInstant() {
+    HoodieTable table = mockTableWithPendingInstants("001", "002");
+    HoodieEngineContext context = mock(HoodieEngineContext.class);
+    doReturn(getDefaultStorageConf()).when(context).getStorageConf();
+    HoodieWriteConfig config = mock(HoodieWriteConfig.class);
+    SupportsUpgradeDowngrade helper = mock(SupportsUpgradeDowngrade.class);
+    when(helper.getTable(config, context)).thenReturn(table);
+    when(config.getMarkersDeleteParallelism()).thenReturn(3);
+
+    ZeroToOneUpgradeHandler handler = org.mockito.Mockito.spy(new ZeroToOneUpgradeHandler());
+    org.mockito.Mockito.doNothing().when(handler).recreateMarkers(anyString(), eq(table), eq(context), anyInt());

Review Comment:
   Addressed in d7887698fe76: added static imports for spy and doNothing and removed the fully qualified Mockito calls. The updated module passes Checkstyle.



##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/action/index/TestIndexingCatchupTask.java:
##########
@@ -240,6 +249,121 @@ public void testNoHeartbeat() throws IOException {
     assertTrue(task.awaitInstantCaughtUp(pendingInstantWithNoHeartbeat), "Expected null as the instant's heartbeat has expired.");
   }
 
+  @Test
+  public void testRunSkipsInstantAlreadyCommittedToMetadata() {
+    HoodieInstant instant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.COMMIT_ACTION, "002");
+    HoodieActiveTimeline metadataTimeline = mock(HoodieActiveTimeline.class);
+    when(metadataMetaClient.reloadActiveTimeline()).thenReturn(metadataTimeline);
+    when(metadataTimeline.filterCompletedInstants()).thenReturn(metadataTimeline);
+    when(metadataTimeline.filter(any())).thenReturn(metadataTimeline);
+    when(metadataTimeline.firstInstant()).thenReturn(Option.of(instant));
+
+    RunningIndexingCatchupTask task = runningTask(instant);
+    task.run();
+
+    assertEquals("002", task.currentCaughtupInstant);
+    assertEquals(0, task.writeActionsUpdated.get());
+  }
+
+  @Test
+  public void testRunUpdatesCompletedWriteActionInsideTransaction() {
+    HoodieInstant instant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.REPLACE_COMMIT_ACTION, "002");
+    stubNoCompletedMetadataInstant();
+    RunningIndexingCatchupTask task = runningTask(instant);
+
+    task.run();
+
+    assertEquals(1, task.writeActionsUpdated.get());
+    assertEquals("002", task.currentCaughtupInstant);
+    verify(transactionManager).beginStateChange(Option.of(instant), Option.empty());
+    verify(transactionManager).endStateChange(Option.of(instant));
+  }
+
+  @Test
+  public void testRunUpdatesCleanRestoreAndRollbackActions() throws IOException {
+    HoodieInstant clean = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.CLEAN_ACTION, "002");
+    HoodieInstant restore = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.RESTORE_ACTION, "003");
+    HoodieInstant rollback = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.ROLLBACK_ACTION, "004");
+    stubNoCompletedMetadataInstant();
+    HoodieCleanMetadata cleanMetadata = mock(HoodieCleanMetadata.class);
+    HoodieRestoreMetadata restoreMetadata = mock(HoodieRestoreMetadata.class);
+    HoodieRollbackMetadata rollbackMetadata = mock(HoodieRollbackMetadata.class);
+    HoodieActiveTimeline activeTimeline = mock(HoodieActiveTimeline.class);
+    when(metaClient.getActiveTimeline()).thenReturn(activeTimeline);
+    when(activeTimeline.readRestoreMetadata(restore)).thenReturn(restoreMetadata);
+    when(activeTimeline.readRollbackMetadata(rollback)).thenReturn(rollbackMetadata);
+
+    try (MockedStatic<CleanerUtils> cleanerUtils = mockStatic(CleanerUtils.class)) {
+      cleanerUtils.when(() -> CleanerUtils.getCleanerMetadata(metaClient, clean)).thenReturn(cleanMetadata);
+      runningTask(clean, restore, rollback).run();
+    }
+
+    verify(metadataWriter).update(cleanMetadata, "002");
+    verify(metadataWriter).update(restoreMetadata, "003");
+    verify(metadataWriter).update(rollbackMetadata, "004");
+    verify(transactionManager).endStateChange(Option.of(clean));
+    verify(transactionManager).endStateChange(Option.of(restore));
+    verify(transactionManager).endStateChange(Option.of(rollback));
+  }
+
+  @Test
+  public void testRunRejectsUnexpectedCompletedActionAndReleasesTransaction() {
+    HoodieInstant instant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, HoodieTimeline.SAVEPOINT_ACTION, "002");
+    stubNoCompletedMetadataInstant();
+
+    assertThrows(IllegalStateException.class, () -> runningTask(instant).run());
+    verify(transactionManager).endStateChange(Option.of(instant));
+  }
+
+  @Test
+  public void testAwaitInstantCaughtUpUsesKnownMetadataInstant() {
+    HoodieInstant instant = INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.INFLIGHT, HoodieTimeline.COMMIT_ACTION, "002");
+    RunningIndexingCatchupTask task = new RunningIndexingCatchupTask(
+        metadataWriter, Collections.singletonList(instant), Collections.singleton("002"), metaClient, metadataMetaClient,
+        transactionManager, "001", engineContext, table, heartbeatClient);
+
+    assertTrue(task.awaitInstantCaughtUp(instant));
+    assertEquals("002", task.currentCaughtupInstant);
+  }
+
+  private RunningIndexingCatchupTask runningTask(HoodieInstant... instants) {
+    return new RunningIndexingCatchupTask(
+        metadataWriter, java.util.Arrays.asList(instants), new HashSet<>(), metaClient, metadataMetaClient,
+        transactionManager, "001", engineContext, table, heartbeatClient);
+  }
+
+  private void stubNoCompletedMetadataInstant() {
+    HoodieActiveTimeline metadataTimeline = mock(HoodieActiveTimeline.class);
+    when(metadataMetaClient.reloadActiveTimeline()).thenReturn(metadataTimeline);
+    when(metadataTimeline.filterCompletedInstants()).thenReturn(metadataTimeline);
+    when(metadataTimeline.filter(any())).thenReturn(metadataTimeline);
+    when(metadataTimeline.firstInstant()).thenReturn(Option.empty());
+  }
+
+  static class RunningIndexingCatchupTask extends AbstractIndexingCatchupTask {
+    private final AtomicInteger writeActionsUpdated = new AtomicInteger();
+
+    RunningIndexingCatchupTask(HoodieTableMetadataWriter metadataWriter,
+                               List<HoodieInstant> instantsToIndex,
+                               Set<String> metadataCompletedInstants,
+                               HoodieTableMetaClient metaClient,
+                               HoodieTableMetaClient metadataMetaClient,
+                               TransactionManager transactionManager,
+                               String currentCaughtupInstant,
+                               HoodieEngineContext engineContext,
+                               HoodieTable table,
+                               HoodieHeartbeatClient heartbeatClient) {
+      super(metadataWriter, instantsToIndex, metadataCompletedInstants, metaClient, metadataMetaClient,
+          transactionManager, currentCaughtupInstant, engineContext, table, heartbeatClient);
+    }
+
+    @Override
+    public void updateIndexForWriteAction(HoodieInstant instant) {
+      writeActionsUpdated.incrementAndGet();
+      currentCaughtupInstant = instant.requestedTime();

Review Comment:
   Addressed in d7887698fe76. AbstractIndexingCatchupTask now advances progress after every successfully processed completed action, and RunIndexActionExecutor reads that progress after Future.get() before building HoodieIndexCommitMetadata. The executor regression now runs the production write-stat catch-up task against a real completed 002 commit and asserts indexUptoInstant is 002. The full PR-focused suite passes all 46 tests.



##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/action/index/TestIndexActionExecutors.java:
##########
@@ -0,0 +1,341 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.table.action.index;
+
+import org.apache.hudi.avro.model.HoodieIndexCommitMetadata;
+import org.apache.hudi.avro.model.HoodieIndexPartitionInfo;
+import org.apache.hudi.avro.model.HoodieIndexPlan;
+import org.apache.hudi.client.heartbeat.HoodieHeartbeatClient;
+import org.apache.hudi.client.transaction.TransactionManager;
+import org.apache.hudi.common.engine.HoodieEngineContext;
+import org.apache.hudi.common.model.WriteConcurrencyMode;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieArchivedTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieLockConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.core.transaction.lock.InProcessLockProvider;
+import org.apache.hudi.exception.HoodieIOException;
+import org.apache.hudi.exception.HoodieIndexException;
+import org.apache.hudi.metadata.HoodieTableMetadataUtil;
+import org.apache.hudi.metadata.HoodieTableMetadataWriter;
+import org.apache.hudi.metadata.MetadataPartitionType;
+import org.apache.hudi.storage.HoodieStorage;
+import org.apache.hudi.storage.StoragePath;
+import org.apache.hudi.table.HoodieTable;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.apache.hudi.common.testutils.HoodieTestUtils.INSTANT_GENERATOR;
+import static org.apache.hudi.common.testutils.HoodieTestUtils.getDefaultStorageConf;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockConstruction;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+class TestIndexActionExecutors {
+
+  private HoodieEngineContext context;
+  private HoodieTable table;
+  private HoodieTableMetaClient metaClient;
+  private HoodieTableConfig tableConfig;
+  private HoodieActiveTimeline activeTimeline;
+  private HoodieStorage storage;
+
+  @BeforeEach
+  void setUp() {
+    context = mock(HoodieEngineContext.class);
+    table = mock(HoodieTable.class);
+    metaClient = mock(HoodieTableMetaClient.class);
+    tableConfig = mock(HoodieTableConfig.class);
+    activeTimeline = mock(HoodieActiveTimeline.class);
+    storage = mock(HoodieStorage.class);
+    when(table.getMetaClient()).thenReturn(metaClient);
+    when(table.getActiveTimeline()).thenReturn(activeTimeline);
+    when(table.getInstantGenerator()).thenReturn(INSTANT_GENERATOR);
+    when(table.getStorage()).thenReturn(storage);
+    doReturn(getDefaultStorageConf()).when(storage).getConf();
+    doReturn(getDefaultStorageConf()).when(context).getStorageConf();
+    when(metaClient.getTableConfig()).thenReturn(tableConfig);
+    when(metaClient.getInstantGenerator()).thenReturn(INSTANT_GENERATOR);
+    when(metaClient.getActiveTimeline()).thenReturn(activeTimeline);
+    when(metaClient.reloadActiveTimeline()).thenReturn(activeTimeline);
+    when(tableConfig.getMetadataPartitions()).thenReturn(Collections.emptySet());
+    when(tableConfig.getMetadataPartitionsInflight()).thenReturn(Collections.emptySet());
+  }
+
+  @Test
+  void testScheduleRejectsSingleWriterConfiguration() {
+    HoodieWriteConfig config = HoodieWriteConfig.newBuilder().withPath("/table").build();
+    ScheduleIndexActionExecutor executor = new ScheduleIndexActionExecutor(
+        context, config, table, "002", Collections.singletonList(MetadataPartitionType.COLUMN_STATS), Collections.emptyList());
+
+    HoodieIndexException exception = assertThrows(HoodieIndexException.class, executor::execute);
+    assertTrue(exception.getMessage().contains(HoodieWriteConfig.WRITE_CONCURRENCY_MODE.key()));
+  }
+
+  @Test
+  void testScheduleCreatesPlanAndIsIdempotent() {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant completed = INSTANT_GENERATOR.createNewInstant(
+        HoodieInstant.State.COMPLETED, HoodieTimeline.COMMIT_ACTION, "001");
+    HoodieTimeline completedTimeline = mock(HoodieTimeline.class);
+    when(activeTimeline.getContiguousCompletedWriteTimeline()).thenReturn(completedTimeline);
+    when(completedTimeline.lastInstant()).thenReturn(Option.of(completed));
+    ScheduleIndexActionExecutor executor = new ScheduleIndexActionExecutor(
+        context, config, table, "002", Collections.singletonList(MetadataPartitionType.COLUMN_STATS), Collections.emptyList());
+
+    Option<HoodieIndexPlan> plan = executor.execute();
+
+    assertTrue(plan.isPresent());
+    assertEquals(1, plan.get().getIndexPartitionInfos().size());
+    assertEquals(MetadataPartitionType.COLUMN_STATS.getPartitionPath(),
+        plan.get().getIndexPartitionInfos().get(0).getMetadataPartitionPath());
+    assertEquals("001", plan.get().getIndexPartitionInfos().get(0).getIndexUptoInstant());
+    verify(activeTimeline).saveToPendingIndexAction(any(HoodieInstant.class), any(HoodieIndexPlan.class));
+
+    when(tableConfig.getMetadataPartitions()).thenReturn(Set.of(MetadataPartitionType.COLUMN_STATS.getPartitionPath()));
+    assertFalse(executor.execute().isPresent());
+  }
+
+  @Test
+  void testScheduleAbortsWhenPendingPlanCannotBeSaved() {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant completed = INSTANT_GENERATOR.createNewInstant(
+        HoodieInstant.State.COMPLETED, HoodieTimeline.COMMIT_ACTION, "001");
+    HoodieTimeline completedTimeline = mock(HoodieTimeline.class);
+    when(activeTimeline.getContiguousCompletedWriteTimeline()).thenReturn(completedTimeline);
+    when(completedTimeline.lastInstant()).thenReturn(Option.of(completed));
+    doThrow(new HoodieIOException("write failed"))
+        .when(activeTimeline).saveToPendingIndexAction(any(HoodieInstant.class), any(HoodieIndexPlan.class));
+
+    try (MockedStatic<HoodieTableMetadataUtil> metadataUtil = mockStatic(HoodieTableMetadataUtil.class)) {
+      metadataUtil.when(() -> HoodieTableMetadataUtil.getInflightAndCompletedMetadataPartitions(tableConfig))
+          .thenReturn(Collections.emptySet());
+      metadataUtil.when(() -> HoodieTableMetadataUtil.metadataPartitionExists(
+          metaClient.getBasePath(), context, MetadataPartitionType.COLUMN_STATS.getPartitionPath())).thenReturn(false);
+
+      Option<HoodieIndexPlan> result = new ScheduleIndexActionExecutor(
+          context, config, table, "002", Collections.singletonList(MetadataPartitionType.COLUMN_STATS), Collections.emptyList()).execute();
+
+      assertFalse(result.isPresent());
+      verify(activeTimeline).deleteInstantFileIfExists(INSTANT_GENERATOR.getIndexRequestedInstant("002"));
+    }
+  }
+
+  @Test
+  void testRunRejectsInvalidConfigurationAndMissingInstant() {
+    RunIndexActionExecutor invalidExecutor = new RunIndexActionExecutor(
+        context, HoodieWriteConfig.newBuilder().withPath("/table").build(), table, "002");
+    assertThrows(HoodieIndexException.class, invalidExecutor::execute);
+
+    HoodieWriteConfig config = multiWriterConfig();
+    when(activeTimeline.filterPendingIndexTimeline()).thenReturn(activeTimeline);
+    when(activeTimeline.filter(any())).thenReturn(activeTimeline);
+    when(activeTimeline.lastInstant()).thenReturn(Option.empty());
+    assertThrows(HoodieIndexException.class,
+        () -> new RunIndexActionExecutor(context, config, table, "002").execute());
+  }
+
+  @Test
+  void testRunRejectsUnreadableAndEmptyPlans() throws IOException {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant requested = requestedIndexInstant();
+    stubRequestedIndexInstant(requested);
+    doThrow(new IOException("read failed")).when(activeTimeline).readIndexPlan(requested);
+    assertThrows(HoodieIndexException.class,
+        () -> new RunIndexActionExecutor(context, config, table, "002").execute());
+
+    doReturn(new HoodieIndexPlan(1, Collections.emptyList())).when(activeTimeline).readIndexPlan(requested);
+    assertThrows(HoodieIndexException.class,
+        () -> new RunIndexActionExecutor(context, config, table, "002").execute());
+  }
+
+  @Test
+  void testRunRejectsPartitionThatAlreadyExists() throws IOException {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant requested = requestedIndexInstant();
+    stubRequestedIndexInstant(requested);
+    HoodieIndexPartitionInfo info = new HoodieIndexPartitionInfo(
+        1, MetadataPartitionType.COLUMN_STATS.getPartitionPath(), "001", Collections.emptyMap());
+    when(activeTimeline.readIndexPlan(requested)).thenReturn(new HoodieIndexPlan(1, Collections.singletonList(info)));
+    when(tableConfig.getMetadataPartitions()).thenReturn(Set.of(MetadataPartitionType.COLUMN_STATS.getPartitionPath()));
+
+    assertThrows(HoodieIndexException.class,
+        () -> new RunIndexActionExecutor(context, config, table, "002").execute());
+  }
+
+  @Test
+  void testRunInitializesFilesPartitionAndCompletesIndexInstant() throws IOException {
+    HoodieWriteConfig config = multiWriterConfig();
+    HoodieInstant requested = requestedIndexInstant();
+    stubRequestedIndexInstant(requested);
+    HoodieIndexPartitionInfo info = new HoodieIndexPartitionInfo(
+        1, MetadataPartitionType.FILES.getPartitionPath(), "001", Collections.emptyMap());
+    when(activeTimeline.readIndexPlan(requested)).thenReturn(new HoodieIndexPlan(1, Collections.singletonList(info)));
+    HoodieTableMetadataWriter writer = mock(HoodieTableMetadataWriter.class);
+    when(table.getIndexingMetadataWriter("002")).thenReturn(Option.of(writer));

Review Comment:
   Addressed in d7887698fe76. The FILES initialization branch now owns the metadata writer with try-with-resources, and the tests verify close() after both successful initialization and a saveAsComplete timeline failure. The full PR-focused suite passes all 46 tests.



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