Skip to content

[hotfix][state/tests] Dispose keyed state backend in testMapStateWithNullValue#28168

Merged
1996fanrui merged 1 commit into
apache:masterfrom
1996fanrui:hotfix-statebackendtestbase-null-value-cleanup
May 16, 2026
Merged

[hotfix][state/tests] Dispose keyed state backend in testMapStateWithNullValue#28168
1996fanrui merged 1 commit into
apache:masterfrom
1996fanrui:hotfix-statebackendtestbase-null-value-cleanup

Conversation

@1996fanrui
Copy link
Copy Markdown
Member

@1996fanrui 1996fanrui commented May 15, 2026

What is the purpose of the change

StateBackendTestBase#testMapStateWithNullValue creates a CheckpointableKeyedStateBackend via createKeyedBackend(...) but never closes/disposes it, leaking the resources held by the backend. Every other test in this file already wraps the body in try { ... } finally { IOUtils.closeQuietly(backend); backend.dispose(); }; this one was missed when it was added in FLINK-38137.

Brief change log

  • Wrap the body of testMapStateWithNullValue in a try { ... } finally { IOUtils.closeQuietly(keyedBackend); keyedBackend.dispose(); } block, matching the cleanup pattern used by every other test in StateBackendTestBase.

Verifying this change

This change is a test-side resource cleanup with no behavior change for production code paths. It can be verified by running StateBackendTestBase and its existing implementations and observing that no test logic is affected.

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

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable

@flinkbot
Copy link
Copy Markdown
Collaborator

flinkbot commented May 15, 2026

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Copy link
Copy Markdown
Contributor

@spuru9 spuru9 left a comment

Choose a reason for hiding this comment

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

Nice catch on this one. While we're here — three other tests in this file have backend.dispose() in their finally but are also missing the IOUtils.closeQuietly(backend) that every other test pairs with
dispose():

  • testKryoRegisteringRestoreResilienceWithRegisteredSerializer (line ~1168)
  • testKryoRestoreResilienceWithDifferentRegistrationOrder (line ~1300)
  • testPojoRestoreResilienceWithDifferentRegistrationOrder (line ~1402)

Pre-existing and out of scope for this hotfix, but pointing out for similar change.

…NullValue

The test creates a CheckpointableKeyedStateBackend via createKeyedBackend
but never closes/disposes it, leaking native resources held by the
backend. For most state backends in the existing suite this is harmless,
but cloud-native backends that own long-lived background tasks (and
poll remote state) keep file handles open under the JUnit @tempdir and
fail class teardown.

Wrap the test body in try/finally with IOUtils.closeQuietly(keyedBackend)
and keyedBackend.dispose(), matching the cleanup pattern used by every
other test in this file.
@1996fanrui 1996fanrui force-pushed the hotfix-statebackendtestbase-null-value-cleanup branch from 4935703 to 42aeab3 Compare May 15, 2026 15:01
@1996fanrui
Copy link
Copy Markdown
Member Author

Thanks @spuru9 for the review, good catch, and updated.

@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label May 15, 2026
Copy link
Copy Markdown
Contributor

@spuru9 spuru9 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member Author

@1996fanrui 1996fanrui left a comment

Choose a reason for hiding this comment

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

Thanks for the review, merging

@1996fanrui 1996fanrui merged commit 80be7e4 into apache:master May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants