hudi-agent commented on code in PR #19077:
URL: https://github.com/apache/hudi/pull/19077#discussion_r3488982881
##########
hudi-timeline-service/src/test/java/org/apache/hudi/timeline/service/functional/TestRemoteHoodieTableFileSystemView.java:
##########
@@ -112,6 +112,23 @@ public void testRemoteHoodieTableFileSystemViewWithRetry()
{
viewWithRetries.getLatestBaseFiles();
}
+ /**
+ * The base class probes {@code AbstractTableFileSystemView.bootstrapIndex}
via reflection to
+ * verify non-bootstrap tables get NoOpBootstrapIndex (avoiding S3 HEAD
probes during init).
+ * {@link RemoteHoodieTableFileSystemView} does not extend {@code
AbstractTableFileSystemView} —
+ * the real view, and hence that field, lives server-side, so the assertion
isn't applicable here.
+ */
+ @Test
+ public void testBootstrapIndexIsNoOpWhenBootstrapBasePathAbsent() throws
Exception {
+ // no-op: covered by the base test against the local view
Review Comment:
🤖 nit: empty `@Test` bodies pass silently and can read as
accidentally-incomplete tests. Could you use `@Disabled` (or `@Ignore` if this
is JUnit 4) with a short reason string instead? It's the idiomatic way to
suppress inherited tests and makes the intent immediately obvious to the next
reader.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/table/view/TestHoodieTableFileSystemView.java:
##########
@@ -237,6 +240,60 @@ public void testCloseHoodieTableFileSystemView() throws
Exception {
}
}
+ /**
+ * For non-bootstrap tables, the file system view must avoid instantiating
the real
+ * {@link HFileBootstrapIndex} during init: its constructor issues two
storage.exists()
+ * probes, which translate to billable HEAD requests on S3-like object
stores. The
+ * NoOp substitute keeps {@code useIndex()} returning false without those
probes.
+ */
+ @Test
+ public void testBootstrapIndexIsNoOpWhenBootstrapBasePathAbsent() throws
Exception {
+ closeFsView();
+ cleanMetaClient();
+ // Re-init the table WITHOUT a bootstrap base path.
+ metaClient = HoodieTestUtils.init(tempDir.toAbsolutePath().toString(),
getTableType());
+ basePath = metaClient.getBasePath().toString();
+ assertFalse(metaClient.getTableConfig().getBootstrapBasePath().isPresent(),
+ "Precondition: table must not have a bootstrap base path configured");
+
+ refreshFsView();
+
+ BootstrapIndex index = readBootstrapIndex(fsView);
+ assertTrue(index instanceof NoOpBootstrapIndex,
+ "Expected NoOpBootstrapIndex for non-bootstrap tables to skip
storage.exists() probes, got: "
+ + index.getClass().getName());
+ assertFalse(index.useIndex(), "NoOpBootstrapIndex.useIndex() must be
false");
+ }
+
+ /**
+ * Counterpart of {@link
#testBootstrapIndexIsNoOpWhenBootstrapBasePathAbsent()}:
+ * when a bootstrap base path is configured we must still load the real index
+ * implementation (here {@link HFileBootstrapIndex}).
+ */
+ @Test
+ public void testBootstrapIndexIsLoadedWhenBootstrapBasePathPresent() throws
Exception {
+ closeFsView();
+ cleanMetaClient();
+ metaClient = HoodieTestUtils.init(tempDir.toAbsolutePath().toString(),
getTableType(),
+ BOOTSTRAP_SOURCE_PATH, /* bootstrapIndexEnable = */ true);
+ basePath = metaClient.getBasePath().toString();
+ assertTrue(metaClient.getTableConfig().getBootstrapBasePath().isPresent(),
+ "Precondition: table must have a bootstrap base path configured");
+
+ refreshFsView();
+
+ BootstrapIndex index = readBootstrapIndex(fsView);
+ assertTrue(index instanceof HFileBootstrapIndex,
+ "Expected HFileBootstrapIndex when bootstrap base path is configured
and index is enabled, got: "
+ + index.getClass().getName());
+ }
+
+ private static BootstrapIndex readBootstrapIndex(SyncableFileSystemView
view) throws Exception {
Review Comment:
🤖 nit: `getDeclaredField("bootstrapIndex")` ties the test to a string
literal — if the field is ever renamed, this breaks at runtime rather than
compile time. Have you considered annotating the field with
`@VisibleForTesting` and giving it package-private visibility (or adding a
small package-private getter) to make the coupling explicit and refactor-safe?
<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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
