xiangfu0 opened a new pull request, #19542:
URL: https://github.com/apache/pinot/pull/19542
External query execution cannot call Pinot's cooperative Java accounting
checkpoints while its query thread is outside Java. Calling the existing
checkpoint from a monitor instead samples the monitor thread, and blocking the
shared monitor to honor a pause prevents it from observing resume requests.
This adds an optional, owner-bound `ExternalExecutionSampler` to
`ThreadAccountant`. Supported accountants publish the original platform query
thread's CPU and JVM allocation deltas into the existing tracker and expose a
nonblocking pause observation. Closing the scope on the owner drains in-flight
sampling before the query context is cleared.
### Compatibility and scope
- The SPI addition is a default method returning `null`. Unsupported
accountants and custom subclasses must explicitly implement their policy before
external execution can use them; callers must retain Java checkpoints or reject
that execution path when capture returns `null`.
- Existing Java sampling paths and query accounting totals are retained.
There are no query wire, configuration, segment, or distributed-state changes.
- Synchronous external work on the owner platform thread is included in its
CPU measurement. CPU on external helper threads and native heap allocations are
**not measured** by this hook and require separate accounting. Virtual owners
are rejected by the CPU/allocation tracker.
- This PR supplies the accounting extension point, not an external engine or
its cancellation/deadline implementation. It adds no benchmark source and makes
no performance claim.
### Integration example
Capture after opening the query's accounting context, on the thread that
will execute external work. The example below is schematic: `sharedMonitor`,
`registration`, and `control` are supplied by the consuming engine; closing the
registration must unregister and drain its callback.
```java
ExternalExecutionSampler sampler =
accountant.captureExternalExecutionSampler();
if (sampler == null) {
throw new UnsupportedOperationException("Accountant requires Java
checkpoints");
}
try (sampler) {
var registration = sharedMonitor.register(() -> {
try {
sampler.sampleUsage();
control.setPaused(sampler.isPaused());
} catch (RuntimeException failure) {
control.cancelWithFailure(failure);
}
});
try {
executeSynchronouslyOutsideJava(control);
} finally {
registration.close(); // Unregister and drain before freeing control
state.
sampler.sampleUsage(); // Owner's final sample; try-with-resources still
closes on failure.
}
}
// Only now clear the owner query context and release the external control
state.
```
The monitor must keep observing while execution is paused so it can forward
resume requests. Sampling failures must stop execution and propagate to the
query. The consuming engine remains responsible for deadlines, cancellation,
and its own allocation budget. Do not independently sample/reset/clear the
owning accountant while this scope is active. The same lifecycle is documented
on `ExternalExecutionSampler`.
### Validation
JDK 25:
```text
./mvnw -pl pinot-core -am \
-Dtest=ExternalExecutionSamplerTest,ExternalThreadAccountingTest \
-Dsurefire.failIfNoSpecifiedTests=false install
```
All 16 selected reactor modules succeeded. All 9 new tests passed (3 SPI, 6
core), covering original-thread CPU/allocation attribution, final accounting
cleanup, real pause/resume state, draining and owner-only close, and
unsupported/custom/virtual/foreign-context rejection. These tests exercise the
Java management counters and lifecycle; they do not benchmark or validate an
external engine.
Affected-module Spotless, Checkstyle, license checks, and warning-enabled
`test-compile` passed. The compiler emitted no warnings for the changed classes.
The additional warning-enabled reactor `test-compile` check stops in
unchanged `pinot-segment-local` code at `ZstandardDecompressor.java:51` because
`org.jetbrains.annotations.NotNull` is unavailable. This does not affect the
successful normal reactor install/test run above.
--
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]
