This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 8442911b266 [fix](fe) Close the FileIO of an Iceberg HiveCatalog that
fails to initialize (#70164)
8442911b266 is described below
commit 8442911b266b1a5d01e9cf1f2c932ea778a28b1f
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Fri Nov 6 02:16:40 2026 +0000
[fix](fe) Close the FileIO of an Iceberg HiveCatalog that fails to
initialize (#70164)
### What problem does this PR solve?
Issue Number: None
Related PR: #66913, #66914
Problem Summary:
**In short.** This ports from branch-4.x #66913 the cleanup of an
Iceberg `HiveCatalog` whose initialization fails after it has created
its FileIO: on master, nothing closes that FileIO. With the FileIOs
master configures, the leaked object holds no native resource today, so
this is hygiene that keeps a FileIO with real resources from leaking
once per failed catalog creation.
**Background.** `DorisHiveCatalog` (fe-connector-iceberg, used for
Iceberg on HMS) extends Iceberg's `HiveCatalog` and owns the FileIO the
catalog creates (#66914). In Iceberg 1.12, `HiveCatalog#initialize`
creates the FileIO first, then the Hive client pool, and only then the
`CloseableGroup` that closes the FileIO, the KMS client and the metrics
reporter. #66913, the branch-4.1 port of #66914, closes the FileIO when
initialization fails; master's `DorisHiveCatalog` does not.
**The problem, and what it cost.** When `CachedClientPool` fails (an
unknown element in `client-pool-cache-keys`, a `clients` that is not a
number) or the metrics reporter fails to load (`metrics-reporter-impl`),
the catalog has a FileIO but no group: `HiveCatalog#close` closes
nothing, and `DorisHiveCatalog` has not captured the FileIO yet. The
catalog properties reach Iceberg as the user gave them, and
`IcebergConnector` caches only a catalog that initialized, so each
metadata access on such a catalog creates and drops another FileIO.
Today that is only garbage: master's HMS catalogs use `HadoopFileIO`,
whose `close` does nothing, and `ResolvingFileIO` and `S3FileIO` create
their clients lazily.
**How this PR fixes it.** When `HiveCatalog#initialize` throws,
`DorisHiveCatalog#initialize` closes the FileIO the catalog created and
rethrows; a failure to close is added to the initialization failure as
suppressed. The FileIO is null when loading the FileIO itself failed,
and then there is nothing to close. Not ported from #66913: its
`close()` changes, since Iceberg 1.12 (#69031) closes the FileIO in
`HiveCatalog#close` itself, and its `DorisHadoopCatalog`. The hadoop
flavor has the same order (`HadoopCatalog` creates the FileIO before the
lock manager), but master uses Iceberg's `HadoopCatalog`, and a subclass
would change the catalog-impl mapping without freeing anything today.
**Results.**
| `client-pool-cache-keys` with an unknown element | Before | After |
|---|---|---|
| `DorisHiveCatalog#initialize` | throws `IllegalArgumentException`, the
FileIO stays open | throws `IllegalArgumentException`, the FileIO is
closed |
Co-authored-by: 924060929 <[email protected]>
---
.../doris/connector/iceberg/DorisHiveCatalog.java | 23 ++++++++++-
.../connector/iceberg/DorisHiveCatalogTest.java | 44 ++++++++++++++++++++++
2 files changed, 65 insertions(+), 2 deletions(-)
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/DorisHiveCatalog.java
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/DorisHiveCatalog.java
index 2bef55d15a9..1b4c4faf1b8 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/DorisHiveCatalog.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/DorisHiveCatalog.java
@@ -32,8 +32,27 @@ public class DorisHiveCatalog extends HiveCatalog {
@Override
public void initialize(String name, Map<String, String> properties) {
- super.initialize(name, properties);
- ownedFileIO = extractFileIO();
+ try {
+ super.initialize(name, properties);
+ ownedFileIO = extractFileIO();
+ } catch (RuntimeException | Error failure) {
+ closePartiallyInitializedFileIO(failure);
+ throw failure;
+ }
+ }
+
+ // HiveCatalog creates the FileIO before the client pool and the metrics
reporter, and only the group it builds
+ // after them closes the FileIO, so when one of those fails, nothing else
closes it.
+ private void closePartiallyInitializedFileIO(Throwable
initializationFailure) {
+ try {
+ FileIO fileIO = extractFileIO();
+ // null when loading the FileIO itself failed
+ if (fileIO != null) {
+ fileIO.close();
+ }
+ } catch (Throwable closeFailure) {
+ initializationFailure.addSuppressed(closeFailure);
+ }
}
@Override
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/DorisHiveCatalogTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/DorisHiveCatalogTest.java
index 297d87e53b2..0b1f25815ea 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/DorisHiveCatalogTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/DorisHiveCatalogTest.java
@@ -17,6 +17,7 @@
package org.apache.doris.connector.iceberg;
+import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.InputFile;
import org.apache.iceberg.io.OutputFile;
@@ -24,6 +25,9 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import java.lang.reflect.Field;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
class DorisHiveCatalogTest {
@@ -41,6 +45,46 @@ class DorisHiveCatalogTest {
Assertions.assertEquals(1, fileIO.closeCount);
}
+ // HiveCatalog#initialize creates the FileIO before the client pool, and
an unknown cache key element fails the
+ // client pool after that.
+ @Test
+ void closesFileIOWhenHiveClientPoolInitializationFails() {
+ TrackingFileIO.CLOSE_COUNT.set(0);
+ Map<String, String> properties = new HashMap<>();
+ properties.put(CatalogProperties.FILE_IO_IMPL,
TrackingFileIO.class.getName());
+ properties.put(CatalogProperties.CLIENT_POOL_CACHE_KEYS,
"invalid-key-element");
+ DorisHiveCatalog catalog = new DorisHiveCatalog();
+
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
catalog.initialize("test", properties));
+
+ Assertions.assertEquals(1, TrackingFileIO.CLOSE_COUNT.get());
+ }
+
+ /** Loaded by HiveCatalog through {@code io-impl}, so it is public and
counts its closes statically. */
+ public static class TrackingFileIO implements FileIO {
+ private static final AtomicInteger CLOSE_COUNT = new AtomicInteger();
+
+ @Override
+ public InputFile newInputFile(String path) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public OutputFile newOutputFile(String path) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void deleteFile(String path) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void close() {
+ CLOSE_COUNT.incrementAndGet();
+ }
+ }
+
private static final class RecordingFileIO implements FileIO {
private int closeCount;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
