morningman opened a new pull request, #70164:
URL: https://github.com/apache/doris/pull/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 |
Original author: @924060929
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
New
`DorisHiveCatalogTest#closesFileIOWhenHiveClientPoolInitializationFails` (from
#66913) has the catalog load a FileIO that counts its closes, through
`io-impl`, and fail on an unknown cache key element: the FileIO is closed once.
Without the change it is never closed (`expected 1 but was 0`). The other case
of the class passes, and fe-connector-iceberg Checkstyle reports nothing.
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
