chore: load Comet on demand

[comphead]

Which issue does this PR close?

Closes #5274

Rationale for this change

Fixes a misleading log line where the Rust "Comet native library version ... initialized" INFO appeared even when Comet was disabled (via spark.comet.enabled=false or the plugin's off-heap gate). The line
came from the JNI init call, which was fired unconditionally by NativeBase's eager static initializer. The static initializer ran on any first touch of NativeBase — including the plugin's shutdown path
(NativeBase.releaseNative()) — so users who had opted out of Comet still saw the message.

The fix makes native-library loading lazy: it now happens only when a Comet code path that actually needs JNI runs. The message is preserved for users who do use Comet; it just no longer fires when Comet is
disabled.

Changes

spark/src/main/java/org/apache/comet/NativeBase.java

• Removed the static initializer's load() call.
• Added a new static block that sets the Arrow system properties (arrow.enable_unsafe_memory_access, arrow.enable_null_check_for_get) at NativeBase class-load time. Arrow latches these in
  BoundsChecking / NullCheckingForGet static initializers, so writing them later is silently ignored. Keeping this in a static block (independent of load()) preserves the previous ordering guarantee.
  These are pure JVM properties and do not require the native library.
• Added a protected NativeBase() constructor that calls a new ensureLoaded(). This makes new Native() — the entry point for every native code path — the natural trigger for the load. Documented at the
  constructor that the library loads at most once per JVM per classloader (both ensureLoaded() and load() short-circuit on loaded).
• isLoaded() now calls ensureLoaded(), so CometSparkSessionExtensions.isCometLoaded also triggers the load lazily. That method is already gated on COMET_ENABLED and platform/shuffle checks, so the
  load only fires when Comet is actually enabled.
• releaseNative() reads the loaded field directly instead of via isLoaded(), so plugin shutdown does not force a lazy load of a library that was never used. Dropped throws Throwable and wrapped
  release() in try/catch — shutdown never propagates.
• setLoaded() now also resets loadErr. Without this, a test that forces a load failure (e.g. os.name=foo in CometSparkSessionExtensionsSuite) leaves a sticky throwable that later isLoaded() calls
  rethrow, breaking downstream tests in the same JVM.
• isFeatureEnabled and isObjectStoreSchemeSupported are now Java wrappers that call ensureLoaded() before delegating to private static native methods isFeatureEnabledImpl /
  isObjectStoreSchemeSupportedImpl. This keeps the public API stable and prevents UnsatisfiedLinkError if a caller happens to hit them before any other trigger.

native/core/src/lib.rs

• Renamed the two corresponding JNI symbols to Java_org_apache_comet_NativeBase_isFeatureEnabledImpl and Java_org_apache_comet_NativeBase_isObjectStoreSchemeSupportedImpl to match the Java-side renames.

Design rationale

Spark's DriverPlugin.init javadoc recommends "postponing [expensive operations] until the application has fully started." Deferring native-library load to first use aligns with that guidance and avoids
paying the cost (or emitting the log) in configurations where Comet is registered but not exercised.

NativeBase is loaded at most once per JVM per classloader. The load is triggered by either new Native() (the constructor path used by every executor code path) or NativeBase.isLoaded (used by the
planning-time check). Both go through ensureLoaded(), which is static synchronized and guarded on loaded || loadErr != null. load() itself is static synchronized and guarded on loaded. Concurrent
first-touch callers serialize on the class monitor; the winner sets loaded=true before returning; losers re-check and no-op.

Behavior per configuration:

• spark.plugins=org.apache.spark.CometPlugin unset — NativeBase is never referenced. No log.
• Plugin registered, off-heap and on-heap both disabled — plugin init returns early. Shutdown's releaseNative() returns without loading. No log.
• Plugin registered, off-heap enabled, spark.comet.enabled=false — extension rules return early inside isCometLoaded before touching NativeBase.isLoaded. No log.
• Comet enabled — first isCometLoaded on the driver or first new Native() on the executor triggers the load. One log per JVM.

Compatibility

• NativeBase.releaseNative() loses throws Throwable. Both callers are in Plugins.scala (Scala, no checked-exception impact).
• NativeBase.isFeatureEnabled / NativeBase.isObjectStoreSchemeSupported keep the same signature. The private native implementations were renamed with an Impl suffix; the JNI symbols follow.

[comphead]

please review it carefully

[andygrove]

Flagging one ordering risk with moving load() out of the static initializer, since it's subtle and not visible in the diff.

load() calls setArrowProperties(), which sets arrow.enable_unsafe_memory_access and arrow.enable_null_check_for_get. Arrow reads both in static initializers (BoundsChecking, NullCheckingForGet), so they only take effect if they're set before the corresponding Arrow class is initialized. Setting them afterwards is silently ignored — no warning, no error, just the non-default behavior quietly persisting.

That was previously guaranteed by NativeBase's static block firing on first touch of the class. After this PR the properties are set at the first new Native() instead, so correctness now depends on new Native() running before any Arrow vector work on every path, on every executor.

Small repro against arrow-vector 19.0.0 showing the latch is real:

// vectorFirst: touch an IntVector, then set the property
IntVector v = new IntVector("x", allocator);
v.allocateNew(1); v.set(0, 1); v.get(0); v.close();
System.setProperty("arrow.enable_null_check_for_get", "false");
// -> NULL_CHECKING_ENABLED = true   (property ignored)

// propFirst: set the property, then touch the vector
// -> NULL_CHECKING_ENABLED = false  (property honored)

To be clear about scope, two things bound this:

• Comet relocates Arrow to org.apache.comet.shaded.arrow, so Spark's own Arrow usage can't latch Comet's copy. Only Comet code touching Comet's Arrow matters.
• In the paths I checked the ordering does still hold — e.g. CometExecIterator and CometBlockStoreShuffleReader both construct new Native() immediately before new NativeUtil().

So I'm not claiming a live bug. The concern is that a previously structural guarantee has become an incidental one that nothing enforces or documents: any future path that touches Arrow before instantiating Native would silently lose these settings, and the symptom would be a quiet performance regression rather than a failure. Worth either a comment at the new Native() sites noting the ordering is load-bearing, or setting the Arrow properties somewhere that doesn't depend on native-library load at all — they're pure JVM system properties and don't actually need the library.

Two smaller notes while I was in here:

• ensureLoaded() latches loadErr permanently and setLoaded() doesn't clear it, which breaks it as a test reset. CometSparkSessionExtensionsSuite fails on this branch — the isCometLoaded test sets os.name=foo to force a load failure, caches the throwable, and the next test (isCometLoaded requires CometShuffleManager when shuffle.enabled=true) then rethrows the stale error and fails with was false. I ran it locally with the native lib built: 7/7 on main, 6/7 here, and 7/7 again after adding loadErr = null; to setLoaded.
• The description says Closes #5724, but that issue doesn't exist — I think you want Discuss spark.comet.enabled parameter behavior #5274, otherwise it won't auto-close.

[comphead]

Thanks @andygrove I also see tests failed because Comet initialized multiple times, it should be initialized once. It still may be static but only if Comet is enabled. Moving this to draft for now, it requires more efforts

[andygrove]

This seems like a complex/risky change just to suppress a log line saying that the driver has loaded. Does it matter that the driver loads even if Comet is disabled? If the user doesn't want the driver to load then they could just not specify the driver as a plugin in the first place?

[andygrove]

The value of COMET_DEBUG_ENABLED can change over the lifetime of the driver - not sure of the impact here

[comphead]

This seems like a complex/risky change just to suppress a log line saying that the driver has loaded. Does it matter that the driver loads even if Comet is disabled? If the user doesn't want the driver to load then they could just not specify the driver as a plugin in the first place?

Thanks @andygrove in fact its more than 1 line, so if Comet disabled the static init still calls

NativeBase.load()

which effectively preloads the Comet codebase, but doesn't enable it. It might confuse the user with Comet related entries even if Comet is disabled. Some cases were reported with failing on Comet class linking however comet was disabled in the first place.

However the change is risky, and if you like I can split it into 2 PRs, simple hide message for now, and second discuss how to behave correctly for Comet code eager load.

UPD: it would be good to split the work. It just came to my mind if we dont load Comet eagerly it would be more problematic to enable/disable Comet on the query level, which I personally find very useful.