comphead opened a new issue, #5274:
URL: https://github.com/apache/datafusion-comet/issues/5274
### What is the problem the feature request solves?
The Rust log line
```
INFO core/src/lib.rs: Comet native library version <version> initialized
```
is emitted even when Comet is disabled at the session level (e.g.
`spark.comet.enabled=false`) or when the plugin bails out on the off-heap gate.
The message is misleading because it implies Comet is running
when in fact no plan is being accelerated.
## Steps to reproduce
1. Register the Comet plugin: `--conf
spark.plugins=org.apache.spark.CometPlugin`.
2. Disable Comet at the session level: `--conf spark.comet.enabled=false`
(or leave `spark.memory.offHeap.enabled=false` and
`spark.comet.exec.onHeap.enabled=false` so the plugin's off-heap check fails).
3. Start a Spark shell / submit a job.
4. Observe the driver / executor logs still contain `INFO core/src/lib.rs:
Comet native library version ... initialized`.
## Root cause
`org.apache.comet.NativeBase` has a static initializer that eagerly calls
`load()`, which in turn invokes the JNI
`Java_org_apache_comet_NativeBase_init`. That function unconditionally logs the
"initialized"
message. The static initializer runs whenever the `NativeBase` class is
first touched by the classloader.
Two paths in the disabled configuration still touch `NativeBase`:
- `CometDriverPlugin.shutdown` and `CometExecutorPlugin.shutdown` call
`NativeBase.releaseNative()`. The old implementation called `isLoaded()` first,
which forces class initialization and hence the eager
`load()`.
- Any code that references a static on `NativeBase` (e.g.
`isObjectStoreSchemeSupported` or a `Native` subclass instantiation) also
class-loads `NativeBase` and triggers `load()`.
Because loading is eager, the "initialized" line fires even on paths that
were only meant to check state or release a never-loaded library.
## Proposed fix
Make the native library load lazily. The library is only loaded when a
code path that actually needs JNI executes:
- Remove the static-initializer `load()` call.
- Add a `protected NativeBase()` constructor that calls a new
`ensureLoaded()` helper, so `new Native()` (the subclass used by every native
code path) triggers the load.
- Make `isLoaded()` call `ensureLoaded()` so
`CometSparkSessionExtensions.isCometLoaded` (already gated on `COMET_ENABLED`
etc.) triggers the load only when Comet is confirmed enabled.
- Change `releaseNative()` to read the `loaded` field directly instead of
going through `isLoaded()`, so plugin shutdown never lazy-loads a library that
was never used. Wrap `release()` in try/catch so
shutdown never propagates.
- Wrap the two public static native entry points (`isFeatureEnabled`,
`isObjectStoreSchemeSupported`) so they call `ensureLoaded()` before invoking
the renamed private native methods `nativeIsFeatureEnabled`
/ `nativeIsObjectStoreSchemeSupported`. This keeps the public API stable
while avoiding `UnsatisfiedLinkError`.
- Rename the corresponding JNI symbols in `native/core/src/lib.rs` to
match.
## Expected behavior
- `spark.comet.enabled=false` (or plugin off-heap check failing): no
`Comet native library version ... initialized` line, no native load work on the
driver or on executor plugin shutdown.
- Comet enabled and actually accelerating a plan: the line still appears
the first time a native code path runs, so users retain the "native lib is
live" signal.
### Describe the potential solution
_No response_
### Additional context
_No response_
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
