mateusaubin opened a new pull request, #16861:
URL: https://github.com/apache/iceberg/pull/16861
## What
Make `CachingCatalog` and the Spark catalogs (`SparkCatalog`,
`SparkSessionCatalog` via `BaseCatalog`) implement `Closeable` and
forward `close()` to the catalog they wrap, guarding each hop with an
`instanceof Closeable` check.
## Why
Catalogs that hold network clients rely on `Catalog.close()` to release
those resources. The REST catalog, for example, drains its pooled HTTP
connections on `close()`. That call never reached the wrapped catalog
through the Spark stack:
- `CachingCatalog` wraps a delegate `Catalog` but did not implement
`Closeable`, so closing the wrapper never closed the delegate.
- `SparkCatalog` / `SparkSessionCatalog` (via `BaseCatalog`) were not
`Closeable`, so the underlying Iceberg catalog was never closed.
On a long-running Spark cluster with frequent catalog changes, every
discarded catalog left its client connections and local ports open.
Over time these accumulated and exhausted the available ephemeral
ports, surfacing as `BindException`.
The fix is generic: any closeable backend (REST, JDBC-backed, cloud SDK
catalogs, and `FileIO` implementations holding their own pools) now gets
its `close()` invoked through the same path. Delegates that are not
`Closeable` are skipped by the guard, so custom/in-memory catalogs are
unaffected.
## Changes
- `core`: `CachingCatalog implements Closeable`, forwards `close()` to
its delegate.
- `spark` (v3.5 / v4.0 / v4.1): `BaseCatalog implements Closeable`;
`SparkCatalog` and `SparkSessionCatalog` forward `close()` to their
underlying Iceberg catalog.
## Tests
Added contract tests in `TestCachingCatalog`:
- `closePropagatesToWrappedCatalog` — verifies `close()` reaches a
closeable delegate (Mockito spy on a real `hadoopCatalog()`).
- `closeIsSafeWhenWrappedCatalogIsNotCloseable` — verifies the
`instanceof Closeable` guard makes `close()` a no-op for
non-closeable delegates.
## Notes
This addresses connection/port leakage on the close path only. Catalogs
that are never closed (e.g. orphaned by paths outside our control) are
out of scope here; an autonomous idle/TTL connection eviction mechanism
could be considered as a follow-up.
AI assistance was used to draft the commit message and this PR
description, and to evaluate the completeness of the fix. The
implementation and design (using `Closeable` for parity with existing
in-tree catalogs, and guarding each delegate hop) were reviewed and
verified manually.
--
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]
