wombatu-kun commented on code in PR #20487:
URL: https://github.com/apache/hudi/pull/20487#discussion_r4434674309
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/storage/hadoop/HoodieHadoopStorage.java:
##########
@@ -249,6 +249,19 @@ public List<StoragePathInfo> globEntries(StoragePath
pathPattern)
.collect(Collectors.toList());
}
+ @Override
+ public boolean isEmpty(StoragePath pathPattern) throws IOException {
Review Comment:
Nit: `pathPattern` is not a glob here, since neither `exists` nor
`listLocatedStatus` expands one, so `path` fits better in both `isEmpty`
signatures. The `iterator == null` branch can also go, because
`listLocatedStatus` always returns an iterator.
##########
hudi-io/src/main/java/org/apache/hudi/storage/HoodieStorage.java:
##########
@@ -484,4 +484,20 @@ public List<StoragePathInfo>
listDirectEntries(List<StoragePath> pathList,
public List<StoragePathInfo> globEntries(StoragePath pathPattern) throws
IOException {
return globEntries(pathPattern, e -> true);
}
+
+ /**
+ * Checks whether the directory at the given storage path is empty.
+ * <p>
+ * Implementation should lean towards memory-efficient approaches (such as
lazy-loading
+ * iterators) to avoid loading all file metadata into memory at once, which
could cause
+ * Out-of-Memory (OOM) errors in large directories.
+ * </p>
+ *
+ * @param pathPattern The {@link StoragePath} to check.
+ * @return {@code true} if the path does not exist or contains no
files/directories;
+ * {@code false} otherwise.
+ * @throws IOException If any storage system I/O error occurs.
+ */
+ @PublicAPIMethod(maturity = ApiMaturityLevel.EVOLVING)
+ public abstract boolean isEmpty(StoragePath pathPattern) throws IOException;
Review Comment:
`HudiTrinoStorage` and `HudiTrinoInlineStorage` do not implement this new
abstract method, so `hudi-trino` no longer compiles and
`test-hudi-trino-plugin` is red. Implement it in both, for example
`!fileSystem.listFiles(convertToLocation(path)).hasNext()` in
`HudiTrinoStorage` and a `HoodieNotSupportedException` in the read-only inline
storage, like its `listFiles`.
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/HoodieStreamer.java:
##########
@@ -781,6 +783,10 @@ public StreamSyncService(Config cfg,
HoodieSparkEngineContext hoodieSparkContext
}
}
} else {
+ if (!this.storage.isEmpty(basePath)) {
Review Comment:
`BootstrapExecutor.initializeTable` still refuses any existing target path,
so `--run-bootstrap` on a pre-created empty base path keeps failing with
`target base path already exists`. Should bootstrap get the same allowance,
here or in a follow-up?
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/HoodieStreamer.java:
##########
@@ -757,7 +757,9 @@ public StreamSyncService(Config cfg,
HoodieSparkEngineContext hoodieSparkContext
this.configurationHotUpdateStrategyOpt =
StringUtils.isNullOrEmpty(cfg.configHotUpdateStrategyClass) ? Option.empty() :
ConfigurationHotUpdateStrategyUtils.createConfigurationHotUpdateStrategy(cfg.configHotUpdateStrategyClass,
cfg, properties.get());
- if (this.storage.exists(new StoragePath(cfg.targetBasePath))) {
+ final StoragePath basePath = new StoragePath(cfg.targetBasePath);
+ final StoragePath metaPath = new StoragePath(basePath,
HoodieTableMetaClient.METAFOLDER_NAME);
+ if (this.storage.exists(metaPath)) {
Review Comment:
The `--target-base-path` help text still says an existing path is expected
to be a hoodie table, which is no longer true for an empty directory. Update it
to mention that an empty directory is accepted.
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/StreamSync.java:
##########
@@ -435,6 +437,10 @@ private HoodieTableMetaClient initializeMetaClient(boolean
refreshTimeline) thro
throw e;
}
} else {
+ if (!this.storage.isEmpty(basePath)) {
Review Comment:
`TestHoodieDeltaStreamer.testTableCreation` still expects
`TableNotFoundException` on an empty directory, so `test-utilities` is red.
Point it at a non-empty directory and assert this new refusal, and add a case
that initializes a table on an empty directory.
--
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]
