danny0405 commented on code in PR #19363:
URL: https://github.com/apache/hudi/pull/19363#discussion_r3642687211


##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metadata/TestHoodieBackedTableMetadataWriter.java:
##########
@@ -252,6 +269,239 @@ void testValidateRollbackForMDT() throws Exception {
     assertDoesNotThrow(() -> validateRollbackMethod.invoke(writer, 
instantToRollback));
   }
 
+  @Test
+  void exercisesNoOpAndUnsupportedBaseWriterPaths() throws Exception {
+    // Cover base no-op hooks and fail fast when an indexer is disabled.
+    HoodieBackedTableMetadataWriter<List<HoodieRecord>, List<?>> writer =
+        mock(HoodieBackedTableMetadataWriter.class, CALLS_REAL_METHODS);
+
+    assertDoesNotThrow(() -> writer.buildMetadataPartitions(
+        mock(HoodieEngineContext.class), Collections.emptyList(), "001"));
+    assertThrows(UnsupportedOperationException.class,
+        () -> writer.streamWriteToMetadataTable(null, "001"));
+    assertThrows(UnsupportedOperationException.class,
+        () -> 
writer.secondaryWriteToMetadataTablePartitions(Collections.emptyList(), "001"));
+
+    HoodieWriteConfig metadataWriteConfig = mock(HoodieWriteConfig.class);
+    when(metadataWriteConfig.getBasePath()).thenReturn("/tmp/metadata");
+    writer.metadataWriteConfig = metadataWriteConfig;
+    setField(writer, "enabledIndexerMap", Collections.emptyMap());
+    HoodieIndexPartitionInfo disabledPartition = 
HoodieIndexPartitionInfo.newBuilder()
+        .setVersion(1)
+        
.setMetadataPartitionPath(MetadataPartitionType.COLUMN_STATS.getPartitionPath())
+        .setIndexUptoInstant("001")
+        .build();
+    assertThrows(HoodieIndexException.class,
+        () -> writer.buildMetadataPartitions(
+            mock(HoodieEngineContext.class), 
Collections.singletonList(disabledPartition), "001"));
+  }
+
+  @Test
+  void fallsBackToConfiguredIndexersWhenTableConfigHasNoMetadataPartitions() 
throws Exception {
+    // A fresh table must derive active indexers from writer configuration.
+    HoodieBackedTableMetadataWriter<List<HoodieRecord>, List<?>> writer =
+        mock(HoodieBackedTableMetadataWriter.class, CALLS_REAL_METHODS);
+    HoodieTableMetaClient dataMetaClient = mock(HoodieTableMetaClient.class);
+    HoodieTableConfig tableConfig = mock(HoodieTableConfig.class);
+    when(dataMetaClient.getTableConfig()).thenReturn(tableConfig);
+    when(tableConfig.getMetadataPartitions()).thenReturn(new HashSet<>());
+    
when(tableConfig.getMetadataPartitionsInflight()).thenReturn(Collections.emptySet());
+    writer.dataMetaClient = dataMetaClient;
+    Map<MetadataPartitionType, Indexer> enabledIndexers = new HashMap<>();
+    enabledIndexers.put(MetadataPartitionType.FILES, mock(Indexer.class));
+    setField(writer, "enabledIndexerMap", enabledIndexers);
+
+    assertDoesNotThrow(() -> writer.processAndCommit("001", 
Collections::emptyList));
+
+    HoodieBackedTableMetadataWriter<List<HoodieRecord>, List<?>> 
streamingWriter =
+        mock(HoodieBackedTableMetadataWriter.class, CALLS_REAL_METHODS);
+    HoodieEngineContext engineContext = mock(HoodieEngineContext.class);
+    HoodieData<HoodieRecord> emptyData = mock(HoodieData.class);
+    when(engineContext.<HoodieRecord>emptyHoodieData()).thenReturn(emptyData);
+    streamingWriter.dataMetaClient = dataMetaClient;
+    setField(streamingWriter, "engineContext", engineContext);
+    setField(streamingWriter, "enabledIndexerMap", Collections.emptyMap());
+    assertSame(emptyData, streamingWriter.streamWriteToMetadataPartitions(
+        mock(HoodieData.class), Collections.emptySet(), "001"));
+  }
+
+  @Test
+  void wrapsMetadataReaderAndFileSliceReadFailures() throws Exception {
+    // Reader setup and lazy file listing must preserve the public exception 
contract.
+    HoodieBackedTableMetadataWriter<List<HoodieRecord>, List<?>> writer =
+        mock(HoodieBackedTableMetadataWriter.class, CALLS_REAL_METHODS);
+    writer.dataWriteConfig = 
HoodieWriteConfig.newBuilder().withPath("/tmp/missing-table").build();
+    writer.dataMetaClient = mock(HoodieTableMetaClient.class);
+    Method maybeReinitializeReader =
+        
HoodieBackedTableMetadataWriter.class.getDeclaredMethod("mayBeReinitMetadataReader");
+    maybeReinitializeReader.setAccessible(true);
+    InvocationTargetException readerFailure = assertThrows(
+        InvocationTargetException.class, () -> 
maybeReinitializeReader.invoke(writer));
+    assertTrue(readerFailure.getCause() instanceof HoodieException);
+
+    HoodieBackedTableMetadata metadata = mock(HoodieBackedTableMetadata.class);
+    HoodieTableFileSystemView metadataView = 
mock(HoodieTableFileSystemView.class);
+    HoodieTableMetaClient dataMetaClient = mock(HoodieTableMetaClient.class, 
RETURNS_DEEP_STUBS);
+    
when(dataMetaClient.getActiveTimeline().filterCompletedAndCompactionInstants().lastInstant())
+        .thenReturn(Option.empty());
+    when(metadata.getMetadataFileSystemView()).thenReturn(metadataView);
+    when(metadata.getAllPartitionPaths()).thenThrow(new IOException("listing 
failed"));
+    writer.metadata = metadata;
+    writer.dataMetaClient = dataMetaClient;
+    setField(writer, "metadataView", metadataView);
+    Method getLazyMergedFileSlices =
+        
HoodieBackedTableMetadataWriter.class.getDeclaredMethod("getLazyMergedFileSlices");
+    getLazyMergedFileSlices.setAccessible(true);
+    Lazy<?> lazyFileSlices = (Lazy<?>) getLazyMergedFileSlices.invoke(writer);
+    assertThrows(HoodieIOException.class, lazyFileSlices::get);
+  }
+
+  @Test
+  void detectsEmptyMetadataTimelineAndHandlesMissingMetadataTable() throws 
Exception {
+    // Missing MDT state requires bootstrap without trusting stale table 
config.
+    HoodieBackedTableMetadataWriter<List<HoodieRecord>, List<?>> writer =
+        mock(HoodieBackedTableMetadataWriter.class, CALLS_REAL_METHODS);
+    Method isBootstrapNeeded = HoodieBackedTableMetadataWriter.class
+        .getDeclaredMethod("isBootstrapNeeded", Option.class);
+    isBootstrapNeeded.setAccessible(true);
+    assertTrue((boolean) isBootstrapNeeded.invoke(writer, Option.empty()));
+
+    HoodieTableMetaClient dataMetaClient = mock(HoodieTableMetaClient.class);
+    HoodieTableConfig tableConfig = mock(HoodieTableConfig.class);
+    when(dataMetaClient.getTableConfig()).thenReturn(tableConfig);
+    when(tableConfig.isMetadataTableAvailable()).thenReturn(true);
+    writer.storageConf = 
org.apache.hudi.common.testutils.HoodieTestUtils.getDefaultStorageConf();
+    writer.dataWriteConfig = 
HoodieWriteConfig.newBuilder().withPath("/tmp/data-table").build();
+    writer.metadataWriteConfig = HoodieWriteConfig.newBuilder()
+        .withPath("/tmp/nonexistent-metadata-table")

Review Comment:
   This assertion depends on `/tmp/nonexistent-metadata-table` never being a 
valid Hudi table. `/tmp` is shared across parallel tests and processes, so 
stale or concurrently created state can make `metadataTableExists()` return 
true and fail this test nondeterministically. Please derive both paths from 
`@TempDir` (or mock the meta-client construction) so the missing-table 
precondition is owned by this test.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to