clintropolis commented on code in PR #20074:
URL: https://github.com/apache/druid/pull/20074#discussion_r3973184854
##########
processing/src/main/java/org/apache/druid/common/asyncresource/SettableAsyncResource.java:
##########
@@ -180,7 +191,8 @@ public void setException(Throwable t)
@Override
public synchronized boolean isReady()
{
- return state == State.READY;
+ // a closed resource is complete too (acquisition is over and get()
reports why)
+ return state == State.READY || state == State.CLOSED;
Review Comment:
I changed this to `state != NEW`, because i think that `isReady()` is just
whether or not you should add a callback to let consumer know when can get
something, and so it means that the acquisition is still happening or not (does
a caller need to wait or not), and a released resource does not have to wait. I
think returning true would convey to a consumer incorrectly that they should
add a ready callback for trying to get something already released, which would
fail with the 'Cannot addReadyCallback in state[..]' instead of getting
'Resource has been released'.
--
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]
