[FLINK-40327][state/forst] Close ReadOptions in ForStGeneralMultiGetOperation to fix native memory leak - #28985
Open
YordanPavlov wants to merge 1 commit into
Open
[FLINK-40327][state/forst] Close ReadOptions in ForStGeneralMultiGetOperation to fix native memory leak#28985YordanPavlov wants to merge 1 commit into
YordanPavlov wants to merge 1 commit into
Conversation
…peration to fix native memory leak ForStGeneralMultiGetOperation.process() creates a native ReadOptions per async read batch and never closes it, leaking native memory proportional to async read volume. Wrap it in try-with-resources: multiGetAsList is synchronous within the executor lambda and returns byte[] copies, so nothing referencing the ReadOptions outlives the batch.
Author
|
@flinkbot run azure |
Collaborator
Author
|
Full Flink CI passed on my fork. The two E2E job failures in the first attempt were runner-infra issues — minikube startup and download rate limits — green on rerun. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
ForStGeneralMultiGetOperation.process()allocates anew ReadOptions()(a JNI object owning native memory) for every executed async read batch, and no code path closes it. On a ForSt + async State V2 job the resulting native memory leak is proportional to read volume. On a production workload we measured TaskManager RSS growth of 2.2–3.9 GiB/h, leading to container OOM kills every ~10–14 hours. jemalloc allocation profiling showedJava_org_forstdb_ReadOptions_newReadOptionsdominating the live-allocation diff.This PR closes the per-batch
ReadOptionswith try-with-resources. This is safe because:db.multiGetAsList(readOptions, ...)is synchronous within the executor lambda and returnsbyte[]copies — nothing referencing theReadOptionsoutlives the batch;ReadOptionsas a managed, closeable resource (ForStResourceContainer#getReadOptions, registered inhandlesToClose).Brief change log
ReadOptionsinForStGeneralMultiGetOperation#processin try-with-resources so it is closed after the batch completes.Verifying this change
This change is already covered by existing tests: functional behavior of
process()for value/list/map states is exercised byForStGeneralMultiGetOperationTest(fullflink-statebackend-forstsuite green locally: 979 tests, 0 failures). No new test asserts the closure itself — theReadOptionsis local to the executor lambda, and we preferred not to add a test-only seam to the production code; happy to add one if preferred.Additionally, verified in production (Flink 2.3.0, ForSt async state backend, with this class patched): TaskManager RSS growth dropped from 2.2–3.9 GiB/h to ~20 MiB/h (flat over 19+ hours) with unchanged job output.
Does this pull request potentially affect one of the following parts
@Public(Evolving): noDocumentation