YordanPavlov opened a new pull request, #29430:
URL: https://github.com/apache/flink/pull/29430

   ## What is the purpose of the change
   
   `ForStIncrementalRestoreOperation#restoreTempDBInstance` opens each 
temporary restore DB with `new DBOptions(forstHandle.getDbOptions())` and never 
closes that copy. The native copy shares everything the backend's options 
reference. With managed memory, that includes the slot-shared 
`WriteBufferManager` and, through it, the block cache, so both outlive the 
disposed backend until the TaskManager exits. Temporary DBs are used for 
multi-handle restores (every scale-in and some scale-outs), so each one leaks 
up to one slot's block cache at the next restart on the same TaskManager. In 
our production job that was ~440 MiB of native memory per rescale. Details, 
measurements and standalone reproductions are in 
[FLINK-40952](https://issues.apache.org/jira/browse/FLINK-40952).
   
   ## Brief change log
   
     - `RestoredDBInstance` takes the `DBOptions` copy and closes it after its 
DB.
     - `restoreTempDBInstance` closes the copy if opening the DB fails.
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
     - Added 
`ForStIncrementalCheckpointRescalingTest#testScalingDownReleasesDBOptions` 
(both restore modes). The test sets a logger on every backend's DB options 
through an options factory, restores two backends into one, closes everything, 
and checks that all loggers are garbage collected. A native `DBOptions` that is 
never closed keeps a JNI global reference to its logger, so without the fix the 
restored backend's logger is never collected and the test fails.
     - Manually verified with the MiniCluster reproduction attached to the JIRA 
ticket (a ForSt async-state job restored alternately at p=2 and p=1, under 
jemalloc): stock 2.3.0 leaks ~410 MiB of RSS per scale-in, and with this fix 
that leak is gone.
   
   ## 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: JobManager (and its 
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: yes (ForSt state 
restore on the TaskManager; only the lifetime of the temporary DB's options 
changes)
     - 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
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes (please specify the tool below)
   
   Generated-by: Claude Code (Claude Opus 5.5)
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to