tombentley commented on pull request #9842:
URL: https://github.com/apache/kafka/pull/9842#issuecomment-756637958
@chia7712 thanks for taking a look. I agree that documenting it isn't a
great solution.
Generally speaking, it's an anti-pattern for the user to go running blocking
code in the future completion handler unless guarantees are provided about the
thread on which that code will run (which we can't assume here, since it's
never been documented). It's not just the blocking methods on KafkaFuture which
are problematic, any blocking method would be enough, e.g.
```java
admin.listTopics()...thenApply(topics -> {
Thread.sleep(60_000);
// blocks the AdminClientRunnable preventing timely handling of other
responses
})
```
I don't think running these completion handlers concurrently is a solution
either. It would introduce thread safety issues if completion handlers started
to be executed concurrently, since it could introduce a need for coordination
between handlers which never existed before (even if though the single threaded
behaviour wasn't guaranteed).
So I think the only solution is to log a warning if the execution of a
future completion takes longer that some expected upper bound. (Throwing an
exception isn't possible in the general case, e.g. the `sleep` example above).
We'd need a separate thread to do this, and in the normal case (where there was
no blocking in the handler) this would be effectively doing nothing. I'm happy
to do it, if people think that this is a sufficiently nasty problem to warrant
a thread just for some warn logging.
Or did you have some other idea about how this could be done?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
