gianm commented on code in PR #20074:
URL: https://github.com/apache/druid/pull/20074#discussion_r3974078224


##########
processing/src/main/java/org/apache/druid/common/asyncresource/SettableAsyncResource.java:
##########
@@ -289,6 +314,14 @@ private boolean setInternal(Either<Throwable, 
ResourceHolder<T>> value)
     final List<Runnable> callbacksToFire;
 
     synchronized (this) {
+      // only close() may cancel a resource
+      if (state == State.NEW && value.isError() && value.error() instanceof 
AsyncResourceCanceledException) {

Review Comment:
   I now think we should get rid of the disallowing of 
`AsyncResourceCanceledException` in `setException`. It would require us to a 
check like this to each ready callback that sets a derived resource:
   
   ```java
   if (targetResource.isReady()) {
     // we must have been closed, ignore this callback.
     return;
   }
   ```
   
   Because otherwise calling `targetResource.setException(t)` would hit this 
and throw. Seems awkward to have to special-case this. Better to treat it as a 
regular exception and let it get passed through.



-- 
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]

Reply via email to