Skip to content

[FLINK-40912][Connectors/Kafka] Close the exactly-once writer's AdminClient - #332

Merged
snuyanzin merged 1 commit into
apache:mainfrom
Savonitar:fix/flink-40912-close-admin-client
Oct 8, 2026
Merged

snuyanzin merged 1 commit into
apache:mainfrom
Savonitar:fix/flink-40912-close-admin-client

Conversation

@Savonitar

@Savonitar Savonitar commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

What is the purpose of the change

Fix FLINK-40912: ExactlyOnceKafkaWriter creates an admin client for POOLING transaction cleanup but never closes it. Each writer restart can leak its network thread and associated resources.

Brief change log

  • Close the lazily created admin client after the existing writer cleanup, preserving the cleanup order. The client remains scoped to the writer's lifetime.
  • Add a package-private nullable getter that returns the existing client without creating one. Production callers retain lazy creation through getOrCreateAdminClient().
  • Add separate integration tests for POOLING client cleanup and non-POOLING lazy initialization. POOLING's client accepts requests before writer closure and rejects them afterward with Kafka's shutdown exception and message. The non-POOLING test currently covers INCREMENTING and verifies that no client is created before or during closure.

Verifying this change

  • Red-green verification: removing the admin-client cleanup makes the POOLING test fail because requests still succeed after writer closure. Fallback cleanup in the test prevents a leaked client when an assertion fails.
  • All 30 tests in ExactlyOnceKafkaWriterTest, ExactlyOnceKafkaWriterITCase, and the two architecture test classes pass. Checkstyle and Spotless pass; frozen ArchUnit violation stores are unchanged.

Does this pull request potentially affect one of the following parts:

  • Dependencies: no.
  • Public API, Table options or PyFlink wrappers: no.
  • Checkpointed state, serializers or exactly-once delivery: yes, the exactly-once writer's resource cleanup is affected. Checkpoint state, transaction selection and commit/abort behavior are unchanged.
  • Per-record code paths: no.

Documentation

  • New feature: no.
  • Feature documentation: not applicable; the private field Javadoc is clarified.
  • English and Chinese user documentation: not applicable.

Was generative AI tooling used to co-author this PR?
  • Yes

Generated-by: Codex (GPT-6)

@Savonitar
Savonitar force-pushed the fix/flink-40912-close-admin-client branch 3 times, most recently from e11b610 to c2ed2fe Compare October 5, 2026 16:32
@Savonitar
Savonitar marked this pull request as ready for review October 5, 2026 16:36

@Efrat19 Efrat19 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this leak! Mostly LGTM

@Savonitar
Savonitar force-pushed the fix/flink-40912-close-admin-client branch from f4abc3e to 413ff1c Compare October 7, 2026 15:59
@Savonitar
Savonitar requested a review from Efrat19 October 7, 2026 17:02
@Savonitar
Savonitar requested a review from Efrat19 October 8, 2026 12:17

@Efrat19 Efrat19 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

…Client

Close the lazily created admin client after the existing writer cleanup
so POOLING writer restarts do not leak its network thread and resources.

Verify that POOLING releases its admin client and INCREMENTING closes
without creating one.

Generated-by: Codex (GPT-6)
@Savonitar
Savonitar force-pushed the fix/flink-40912-close-admin-client branch from 0327bee to 51623cf Compare October 8, 2026 12:52
@snuyanzin
snuyanzin merged commit e47cf42 into apache:main Oct 8, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants