FrankChen021 commented on code in PR #20074:
URL: https://github.com/apache/druid/pull/20074#discussion_r3813151461
##########
processing/src/main/java/org/apache/druid/common/asyncresource/SettableAsyncResource.java:
##########
@@ -308,6 +315,17 @@ private boolean setInternal(Either<Throwable,
ResourceHolder<T>> value)
canceler = null;
callbacksToFire = drainCallbacks();
}
+
+ if (!didSet && value.isError()) {
+ // Nothing will ever surface this error: get() on a closed resource
throws "Closed" and the callbacks are gone,
+ // so log it rather than let a failure that lost a race with close()
vanish
+ if (value.error() instanceof CancellationException) {
+ LOG.debug(value.error(), "Resource canceled by close().");
+ } else {
+ LOG.warn(value.error(), "Resource failed after close().");
Review Comment:
[P2] Cancellation races now emit WARN logs
PartialQueryableIndexCursorFactory intentionally calls setException after
cancellation and documents the closed-resource path as a silent no-op. A queued
producer that observes the resource as closed now produces a WARN with a stack
trace for normal cancellation, adding noisy false alarms to an expected race.
##########
processing/src/main/java/org/apache/druid/common/asyncresource/SettableAsyncResource.java:
##########
@@ -263,9 +268,11 @@ public void close()
default -> throw DruidException.defensive("Already closed");
};
- // Clear result and canceler to allow GC.
+ // Clear result and canceler to allow GC. Dropping the pending ready
callbacks ensures they are not fired if
+ // close() was called before set/setException.
result = null;
canceler = null;
+ readyCallbacks.clear();
Review Comment:
[P1] Closing can strand await callers
AsyncResource.await() registers a latch callback and blocks. Clearing
callbacks on close removes that wake-up path; if close wins before completion,
a late set or setException drains nothing, so an unbounded await blocks
indefinitely and a timed await waits until its timeout. Add a close-aware
wake-up path and a regression test.
--
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]
