Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Future: detect blocking calls in methods that return Futures #76

Open
vlsi opened this issue Dec 25, 2019 · 6 comments
Open

Future: detect blocking calls in methods that return Futures #76

vlsi opened this issue Dec 25, 2019 · 6 comments

Comments

@vlsi
Copy link

vlsi commented Dec 25, 2019

Motivation

https://twitter.com/leventov/status/1209782345756270592
https://cwiki.apache.org/confluence/display/KAFKA/KIP-286%3A+producer.send%28%29+should+not+block+on+metadata+update

Desired solution

It should print an error or raise an exception if a blocking call is executed from a method that returns Future.

Considered alternatives

Inspect code manually

@leventov
Copy link

This may also apply when a method returns other future-like objects: org.reactivestreams.Publisher, j.u.c.Flow.Publisher, RxJava's Observable.

@bsideup
Copy link
Contributor

bsideup commented Dec 25, 2019

‪BlockHound already detects this Kafka's issue:‬

https://speakerdeck.com/bsideup/geekout-2019-dont-be-homer-simpson-with-your-reactor?slide=116‬

‪There is no need to support "methods that return Future" because BlockHound operates on Threads level‬ :)

@vlsi
Copy link
Author

vlsi commented Dec 25, 2019

Oh.

However, someone (in their enterprise app) might still use Futures (e.g. for their do it yourself concurrency). In that case, BlockHound might spot that the app is using an unexpected call sequence.

@leventov
Copy link

Pardon a little Scala code...

def myMethod(): Future[Result] = {
  try {
    val res: Result = rpc()
    successful(res)
  } catch {
    case e: Exception => failed(e)
  }
}

Code pretty much like this was an inspiration for the checklist item.

@bsideup
Copy link
Contributor

bsideup commented Dec 25, 2019

@vlsi

BlockHound works with any JVM code, Futures included.

@leventov
if myMethod is called from a non-blocking thread then it will be reported by the current version of BlockHound.

@leventov
Copy link

@bsideup even if we are already in a thread pool designed for blocking operations, this may introduce unnecessary delay: imagine we want to parallelize execution of myMethod() and myMethod2():

myMethod().thenAcceptBoth(myMethod2(), (res, res2) -> { ... });

This code will unnecessarily wait for the blocking operation in the beginning of myMethod() before it even calls to myMethod2() which fires another async operation. This delay could have been avoided if myMethod() returned more promptly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants