Repository navigation
Flink: Reuse loaded Catalog in TableSerializerCache instead of reloading per cache miss (cherry-pick apache/iceberg#17437) - #14
Conversation
…ing per cache miss catalogLoader.loadCatalog().loadTable(...) reflectively instantiates a new Catalog and calls initialize() on every cache miss. For REST catalogs this builds and stores a real HTTPClient (connection pool) that's never closed, leaking one HTTP client per schema-evolution event on a long-running job. Cache and reuse the loaded Catalog for the serializer's lifetime instead. Cherry-picked from apache#17437.
Cherry-picked from apache#17590, a real, already-merged upstream v2.0/v1.20 backport of apache#17437 — not a from-scratch port.
|
it looks like this may have merged or rebased rather than cherry-picking, its showing a 3000+ file diff |
Fixed now. Was pointing at |
dcagney
left a comment
dcagney
left a comment
There was a problem hiding this comment.
Approving — diff matches upstream apache#17437 + backport apache#17590 byte-for-byte across flink/v1.20, v2.0, v2.1 (v2.0 compared against upstream's historical pre-fix state, since that module has since been dropped upstream — expected, not drift).
Checked specifically for hidden dependency on unbackported upstream work: CatalogLoader, RESTSessionCatalog, and the serializer duplicate()/snapshot-restore contracts this fix touches are all unmodified between the relevant upstream commits and current HEAD — no missing prerequisite.
LGTM.
Bumps version.txt and adds the AFFIRM_VERSIONS.md changelog entry for the next patch release, covering the 3 review-triggered fixes from all-the-things#222721 (#12/#13/#14). Those PRs are open, not yet merged — rebase this branch onto 1.11.0-affirm-patch once they land, then tag 1.11.0-affirm-patch.2 at the resulting commit and publish per RUNBOOK_ICEBERG_PATCH_PUBLISH.md.
* Document and stage 1.11.0-affirm-patch.2 Bumps version.txt and adds the AFFIRM_VERSIONS.md changelog entry for the next patch release, covering the 3 review-triggered fixes from all-the-things#222721 (#12/#13/#14). Those PRs are open, not yet merged — rebase this branch onto 1.11.0-affirm-patch once they land, then tag 1.11.0-affirm-patch.2 at the resulting commit and publish per RUNBOOK_ICEBERG_PATCH_PUBLISH.md. * Update AFFIRM_VERSIONS.md: base off 1.11.0-affirm-patch.1, drop reviewer names * Drop #10/#11 from .2 table (already covered by .1 base) * Note #12/#13/#14 merged and branch rebased; ready to tag * Drop 'not yet published' framing now that .2 contents are final * Drop trailing note from .2 section to match .1 format
What
TableSerializerCache.SerializerInfo.update()callscatalogLoader.loadCatalog().loadTable(...)on every cache miss.CatalogLoader.loadCatalog()reflectively instantiates a newCatalogand callsinitialize()— for REST catalogs,RESTSessionCatalog.initialize()builds and stores a realHTTPClient(connection pool) that this path never closes.Why this matters for us
replication-kt'sDynamicIcebergSink.forInput(...).catalogLoader(CatalogBuilders.polarisCatalogLoader())(all 3 apps) hits this cache-miss path on every schema-evolution event — a real, recurring trigger, not theoretical. Each miss leaks one REST HTTP client/connection pool, accumulating over a long-running job's life. Pre-existing onmaster/1.10.2 today — flagged by dcagney reviewingall-the-things#222721.Fix
Cache the loaded
Catalogand reuse it instead of reloading on every miss.flink/v2.1.flink/v1.20andflink/v2.0via the real, already-merged upstream backport Flink: Backport dynamic sink serializer cache catalog reuse to 2.0 and 1.20 (#17437) apache/iceberg#17590 — not a from-scratch port.