linliu-code opened a new pull request, #19998: URL: https://github.com/apache/hudi/pull/19998
### Describe the issue this Pull Request addresses `RocksDBDAO.init()` registers an anonymous `org.rocksdb.Logger` into a method-local `DBOptions`. RocksDB's native `LoggerJniCallback` is owned through a `std::shared_ptr` and keeps a **JNI global reference** to the Java `Logger` until the last copy of that pointer is released. Nothing released any of them, so every `RocksDBDAO` ever constructed stayed reachable from a JNI global root for the life of the JVM. Observed on a long-lived Spark driver: 458 `RocksDBDAO$1` instances retaining 9.51 GB, 37% of a 23.7 GB live set, each rooted one hop from a JNI Global with no Java owner, ending in `OutOfMemoryError: GC overhead limit exceeded`. ### Summary and Changelog Three separate holders of the callback's `shared_ptr` had to be released: 1. **The `DBOptions` and the `Logger`.** Both were unreachable once `init()` returned. They are now fields, released in `close()`. 2. **The `Options` built in `loadManagedColumnFamilies()`.** `new Options(dbOptions, new ColumnFamilyOptions())` copy-constructs from `dbOptions`, which copies its `info_log` — another `shared_ptr` to the same callback — and it was never closed. **On its own this kept the callback alive with every other handle reading as closed**, so fixing (1) alone did not make the Logger collectable. Both are now closed via try-with-resources. 3. **Everything opened before a failure inside `init()`.** `init()` runs from the constructor, so a throw leaves no reference for any caller to `close()` — including an already-open `RocksDB` when the validation after `RocksDB.open()` fails. A shared `closeOnInitFailure()` releases the handles, the write options, the DB and the native options before the exception propagates. The anonymous `Logger` was also declared in an instance method, so it captured `RocksDBDAO.this` and the JNI reference pinned the whole DAO — its column-family maps included — rather than just the callback. It is now a static nested class, so a missed `close()` would leak bytes rather than an entire DAO. ### Impact No API or behavior change. A long-running process that creates and closes `RocksDBDAO` instances — a Spark driver holding one `RocksDbBasedFileSystemView` per table — no longer accumulates them. ### Risk Level low Release ordering is not load-bearing: the DB, the `DBOptions`, the `Options` copy and the `Logger` each hold the `shared_ptr`, the last release frees it, and skipping any one pins the Logger. `AbstractImmutableNativeReference.close()` is CAS-guarded and idempotent, so the extra closes are safe on repeated `close()`. Verified against `rocksdbjni` 7.5.3 sources: `Logger.disposeInternal` documents the callback as a `std::shared_ptr`; `Options(DBOptions, ColumnFamilyOptions)` calls `newOptions(dbOptions.nativeHandle_, columnFamilyOptions.nativeHandle_)`, the copy that duplicates `info_log`. ### Documentation Update none ### Contributor's checklist - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute) - [x] Enough context is provided in the sections above - [x] Adequate tests were added if applicable - [x] CI passed ### Testing Three tests, each verified to fail without the change it guards: | Test | Asserts | Without the fix | |---|---|---| | `testCloseMakesLoggerCollectable` | the Logger is collectable after `close()` | `logger is still strongly reachable after close(), so a JNI global reference survived` | | `testClosedDaosDoNotAccumulate` | 20 create/close cycles leave nothing reachable | `20 of 60 objects are still strongly reachable after close()` | | `testRepeatedViewLifecyclesLeaveNothingReachable` | the same through `RocksDbBasedFileSystemView` | `5 of 15 objects opened by the view are still strongly reachable after close()` | | `testInitFailureBeforeOpenReleasesNativeHandles` | `init()` failure before `open()` releases handles | `logger must be closed when init() fails` | | `testInitFailureAfterOpenReleasesDatabaseAndHandles` | and after `open()`, including the DB | `an opened RocksDB must be closed when init() fails after it` | Asserting handle state alone is not enough — `isOwningHandle()` reads `false` while the object is still pinned — and asserting on the DAO or the view is not enough either, since the Logger no longer captures its DAO. The weak-reference tests are what actually pin the claim. `mvn test -pl hudi-hadoop-common -am -Dspark3.5 -Dscala-2.12 -Dflink1.19 -Dtest='TestRocksDBDAO,TestRocksDbBasedFileSystemView'` — 14/14 and 52/52, checkstyle clean. 🤖 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]
