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

Coroutine support #94

Closed
bdueck opened this issue Jan 27, 2018 · 28 comments
Closed

Coroutine support #94

bdueck opened this issue Jan 27, 2018 · 28 comments

Comments

@bdueck
Copy link

bdueck commented Jan 27, 2018

Are there plans to support coroutines in http4k?

Specifically, would be great to see an enhanced HttpAsyncClient interface and implementations that offer a suspend function to be used in place of the current invoke with callback. I've prototyped this and seems to work well.

On the http server side, it would be nice to see route support that would allow async responses to be provided. Currently routes are called on the netty worker thread - and if those threads block on IO or otherwise take a long time to complete, then worker threads are unavailable to service other requests. Ideally there would be a coroutine friendly way of specifying routes - allowing the worker thread to go back to its business of dispatching work while leaving the drudgery of creating the response up to coroutines and the threads in their associated contexts. Haven't prototyped this yet - but worming my way through http4k code and have an idea of where this might work.

@daviddenton
Copy link
Member

This is something that we envisage for the longer-term, but haven't really looked at yet due to a couple of things:

  1. Co-routines are still marked as experimental
  2. Opportunity to really look at it

Absolutely the most important thing for us is definitely developer experience - as per "Server as a Function", we definitely want the APIs for both server and client to be identical as this enables a lot of the conveniences that you get with http4k (some of them are explained in this presentation if you're not familar) .

TBH, the current async client API was really a bit of a special case for us to enable fire and forget messages - that's why it's called AsyncHttpClient and not AsyncHttpHandler. There's obviously a lot of crossover between an async and async implementations, but I'd hazard that we would probably end up creating 2 different versions of the core - not sure how this would affect the Filter interface since a lot of the sub-modules provide filters as a part of their implementation.

Anyway - we're very happy for people to do some investigations to see what it might look like if we did introduce co-routine support - so thanks for your efforts! :)

@IgorPerikov
Copy link
Contributor

not experimental anymore 🎉
https://blog.jetbrains.com/kotlin/2018/10/kotlin-1-3/

@alisabzevari
Copy link

I've investigated a little bit regarding this issue and started from changing this:

typealias HttpHandler = (Request) -> Response

to this:

typealias HttpHandler = suspend (Request) -> Response

This change causes some issues which mainly related to the fact that Kotlin does not support suspending functions as supertypes. Here are different cases I found in the code:

Classes which implement HttpHandler

One example is JavaHttpClient. In these cases we should try to not change API surface of http4k.

I've investigated these solutions so far:

  1. Convert the JavaHttpClient class to a function. This will change the api surface of the library:

Example:

Before:

val client = awsClientFilter(Payload.Mode.Unsigned)
     .then(JavaHttpClient())

After:

val client = awsClientFilter(Payload.Mode.Unsigned)
     .then(::javaHttpClient)
  1. Remove HttpHandler as superclass and change invoke to a suspending function. This will also change the api surface of the library:

Example:

After:

val client = awsClientFilter(Payload.Mode.Unsigned)
     .then(JavaHttpClient()::invoke)
  1. Convert the JavaHttpClient class to a function that returns a suspending function.

API surface will not change (except that the returning function is a suspending one). This is the best solution so far!

Example:

Implementation:

fun JavaHttpClient(): HttpHandler = { request: Request ->
    ...
}

Usage:

val client = awsClientFilter(Payload.Mode.Unsigned)
    .then(JavaHttpClient())

Classes that will extend HttpHandler in user code

I think nothing will change except that the framework user cannot extend HttpHandler.

Example:

Before:

class HealthcheckHandler: HttpHandler {
    override fun invoke(request: Request): Response {
        return Response(Status.OK)    
    }
}

...  
    // somewhere in the router
    "/healthcheck" bind Method.GET to HealthcheckHandler()

After:

class HealthcheckHandler {
    override fun invoke(request: Request): Response {
        return Response(Status.OK)    
    }
}
...  
    // somewhere in the router
    "/healthcheck" bind Method.GET to HealthcheckHandler()

interfaces implementing HttpHandler

In http4k a lot of things are actually HttpHandlers.

I don't see any good solutions for this and I think this is the most challenging part of this migration.

Other important types that should be changed

It looks like Filters should become suspending, WsHandler should become a Channel (something similar) and lenses should be able to extracted and injected asynchronously.

But, to make the development simpler, maybe it makes sense to start working on HttpHanlder, then incrementally add other async features to other concepts.

If the result of this investigation is helpful I can continue investigating on other parts of the codebase as well.

@daviddenton
Copy link
Member

That's great investigating. Thank you for putting in the time! 👏👏

I've also been playing around and have come to a few of the same conclusions.

In the short term, we can fix up a few things in master to allow us to make migration easier.

  1. We can use the "object with invoke()" trick to remove a lot of the client classes from extending the HttpHandler. This should have no API changes (unless people are directly referring to JavaHttpClient etc which they shouldn't anyway 🙃).

  2. Another thing is to move the tests to a testing framework which will allow us to test suspend functions natively - I suspect that KotlinTest had something for this. Need to investigate coverage compatibly for this over.

As for the API changes, I think I managed to get the entire prod code compiling in a spike branch (it went on to compiling tests and complained due to calling suspend functions from non) so that is encouraging.

The lack of inheritance with suspend functions is a bit of an issue, and I think we may have to resort to defining "HttpHandler { Response...}" instead of raw functions. It seems to be either that or having { Response... }::invoke passed to the routers whixh sucks. There might also be an issue with RoutingHttpHandlers.

@alisabzevari
Copy link

alisabzevari commented Oct 31, 2018

About tests, we can use a helper function like this:

fun testAsync(block: suspend CoroutineScope.() -> Unit) = runBlocking(block = block)

and write test functions like this:

fun `a simple async test`() = testAsync {
  delay(100)
  ...
}

Although I checked kotlintest and its a pretty nice framework and supports testing suspending functions out of the box.

If you decided to use kotlintest, I am ready to pick this migration task. I think it is better to do this migration before starting coroutinification of http4k.

@daviddenton
Copy link
Member

we definitely want something that does sync and asynchronous stuff without custom infra. Then it'll be easier to do a bulk search/replace in one go.

We need to decide if KT is good enough for us and need to have a proper look before we pull the trigger, so hang fire on anything further until we decide. Thanks v much for the offer tho 🙏🙏

@daviddenton
Copy link
Member

daviddenton commented Dec 19, 2018

I've been toying with this a bit more - I've managed to migrate the API to use coroutines basically without any significant changes .

The main difference is that HttpHandler is now an interface (because you can't inherit from suspending functions). However, you only need to actually declare something as an HttpHandler if your creating it from scratch as a val (like with Filter) - routing blocks still take the function form. Which I'm pretty happy with. You can see that new form here: https://github.com/http4k/http4k/blob/v4_spike/src/docs/cookbook/server_as_a_function/example.kt

Check out the v4_spike branch to see how it looks.

@sshquack
Copy link

@daviddenton The v4_spike branch looks awesome. Looking forward to a release version with coroutines. 🙏

@sshquack
Copy link

@s4nchez @daviddenton Any word on the coroutine support in http4k yet? Kotlin coroutines have been marked stable for a while now. It would be great for users to have suspending function support in http4k.

@michaelbannister
Copy link
Contributor

Do you still want help with this? I'm by no means an expert (with either coroutines or http4k) but if there is gruntwork to go through converting tests or anything I'd be happy to help out. :)

@daviddenton
Copy link
Member

@michaelbannister Thank you very much for the offer! :)

We do have the v4_spike branch where all of the easy changes have already been made - the things that remain are, unfortunately, the complex bits around plugging in the client and server integrations (apart from the Ktor integration which is a no-op).

@zvozin
Copy link
Contributor

zvozin commented Apr 16, 2020

+1 on helping out. Getting on right about now with building something that will probably require non-blocking to scale. Would hate to have to switch frameworks - http4k ftw.

@pamu
Copy link

pamu commented Jul 7, 2020

+1 Wating for the branch to get merged into master. Thanks for the awesome work!

@pamu
Copy link

pamu commented Jul 10, 2020

Can I know what is stopping this branch from getting merged into master? Is there any way I can help to fasten this process? (Not an expert on htt4k but eager to help with enough pointers in right direction.)

@s4nchez
Copy link
Collaborator

s4nchez commented Jul 10, 2020

The branch is nowhere near production ready.

The individual clients and servers need to be rebuilt to work with coroutines. We also need to assess the full impact of turning HttpHandler into an interface with suspend.

We’re happy for people to explore the problems, but I don’t feel like we’re near merging anything around this support at this stage.

@pamu
Copy link

pamu commented Jul 10, 2020

What is the current strategy for dealing with blocking IO calls in http4k? Is there any work-around for now?

@s4nchez
Copy link
Collaborator

s4nchez commented Jul 10, 2020

@pamu do you mean in the context of a coroutine? I don't have a good answer to translate from InputStreams (which is what most client/servers use) to something like Flows, but would be very curious to see how it could look like.

@pamu
Copy link

pamu commented Jul 10, 2020

@s4nchez

val register: HttpHandler = { request ->
  val user = Body.auto<User>.toLens().extract(request)
  dao.createUser(user) // Blocking Database call which blocks webserver thread
  Response(OK)
}

I would like to go nonblocking and asynchronous by using Futures (Promises) or coroutines. Is this possible with the current version of http4?

@s4nchez
Copy link
Collaborator

s4nchez commented Jul 10, 2020

Blocking Database call which blocks webserver thread

Unless the underlying server supports a different model, there's not much http4k can do.

We've designed it to be compatible with various battle-tested servers and to provide the most straightforward developer experience for people moving to serverless, where each request spawns a whole new, independent "server".

Until we can make async work nicely on top of current clients and servers (or decide to drop support for the ones that won't fit the model), the style you're after is not supported.

@pamu
Copy link

pamu commented Jul 13, 2020

@s4nchez I can do the following this Javalin (javalin.io)

import io.javalin.Javalin
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.async
import kotlinx.coroutines.future.asCompletableFuture


fun main(args: Array<String>) {
    
    data class Lang(val name: String, val rank: Int)
    
    val server = Javalin.create()
    server.get("/topLang") {
        val result = GlobalScope
                .async { Lang("Javascript", 1) }
                .asCompletableFuture()
        it.json(result)
    }
    server.start(9000)
}

Output:

{
name: "Javascript",
rank: 1
}

It would be nice if at least CompletableFuture is supported by http4k to make it even better.

http4k => No magic, Only function composition

I would like to work on adding this support. I think http4k is one of the frameworks which takes immutable and testability very seriously. Having this support would make it address a much wider scope of use-cases.

@christophsturm
Copy link

So what about coroutines support? people on the http4k slack asking for coroutines support are referred to it, but after reading through all the comments it's unclear to me if this is still Planned. at some point, there seemed to be some enthusiasm to go the coroutines route, but now reading between the lines it looks to me like nope, not going to happen.
I'm thinking of building my own very simple HTTP abstraction for kotlin, which will probably at the beginning look a lot like "the 30 lines of code" that http4k was at the beginning, but with coroutines. But if the http4k team still has coroutines on the radar i could probably also find a better way to spend my time.

@dsaborg
Copy link

dsaborg commented Feb 6, 2021

So what about coroutines support?

I'm also wondering, it does not seem entirely future-proof to adopt a framework which is based on blocking calls 2021.

@daviddenton
Copy link
Member

daviddenton commented Feb 6, 2021

There is nothing going on with this at the moment. The core team have been working on other things (we all have actual paying jobs 😄), and this just hasn't gotten any attention because it's simply not the most important, or TBH, most interesting thing to be adding value to the library.

There are a few obstacles and barriers to our enthusiasm on this:

  1. The core abstraction of HttpHandler would have to become an interface with a single suspend method instead of a typealias because fundamentally coroutines are a compiler trick and actually not supported by the typesystem - you cannot extend from a suspend function. We have modelled on a spike branch and it is quite easy to do, but it a large, breaking API change.
  2. Once the core abstraction has been put in, we need to work out how to cleanly and simply integrate the coroutine interface with the various servers (and clients) - and there are 17 servers in total to do including the servlet and 6 serverless platforms. Current number of offers of assistance - 0.
  3. Loom - we are unsure of how this might affect the advantage for coroutines on the JVM at the top level.
  4. Multiplatform - this would actually be a reason to move on with the work, but we are still tied to the JVM by the lack of stable IO and DateTime libraries.
  5. Dependencies. Adding coroutines would actually massively increase the weight of the library and bring in several external libraries.
  6. At the moment our focus is actually on Serverless platforms (and associated lightweight) and there are currently zero use-cases for coroutines there at the moment AFAIK.

So - just to bring clarity to this issue, I'm just going to close it for now until either lots of the above things are fixed, or someone actually starts paying us to deliver them.

I'm also wondering, it does not seem entirely future-proof to adopt a framework which is based on blocking calls 2021.

That's entirely your call, but I'd encourage you not to be swayed by the web-scale hype. The initial use-case of Http4k was to run traffic for one of the top 1200 websites in the world, on which it serves a metric load of traffic quite easily on a few nodes (I think they were Jetty IIRC).

tl;dr
If you're looking for coroutine support in February 2021 because your application requires more throughput than the example above, our advice is to choose (or build and maintain) another library.

@lukaszgendek
Copy link

lukaszgendek commented Nov 5, 2021

I've investigated a little bit regarding this issue and started from changing this:

typealias HttpHandler = (Request) -> Response

to this:

typealias HttpHandler = suspend (Request) -> Response

This change causes some issues which mainly related to the fact that Kotlin does not support suspending functions as supertypes.

Suspending functions as supertypes are now available in kotlin 1.6, please check kt-18707.

typealias HttpHandler = suspend () -> String

class HttpHandlerImpl : HttpHandler {
    override suspend fun invoke() = "hello"
}

suspend fun main() {
    println(HttpHandlerImpl().invoke())
    println(KotlinVersion.CURRENT)
}

printing

hello
1.6.0

@peterfigure
Copy link

would be great to revisit this issue

@joostbaas
Copy link

joostbaas commented Jun 7, 2022

At the moment our focus is actually on Serverless platforms (and associated lightweight) and there are currently zero use-cases for coroutines there at the moment AFAIK.

I believe I have a use-case for coroutines on serverless actually. On the server (listener) side indeed not so much, because there are never any concurrent requests. (That seems like inefficient use of hardware, but the whole point is that that is not your problem but someone else's).
Doing concurrent http requests from a serverless application in a non-blocking way would be quite useful though, of course you could just take a bunch of threads and block them (Dispatchers.IO), but when doing that for more than a few handfuls of requests I expect all the context switches would cause a significant performance difference.

@razvn
Copy link
Contributor

razvn commented Jun 7, 2022

Can't you do that by using a runBlocking around the ensemble of your requests and use coroutines for running them ?
Beside, Java's Virtual Threads arrive with Java 19 (in preview) this September. There are already JDK 19 builds with it and some servers/clients already have some early version with support for them, so I think wait & see right now is the right choice.

@joostbaas
Copy link

Can't you do that by using a runBlocking around the ensemble of your requests and use coroutines for running them ?

of course, but they will block the thread they are running on. If you use the Dispatchers.IO dispatcher they will at least block dedicated threads from a thread pool, but you still suffer the additional costs associated with threads.

Using an adapter for a non-blocking AsyncHttpHandler (like OkHttp) using suspendCoroutine works, but with the current API of Filter there is no way (that I know of) to decorate a non-blocking client with for instance ClientFilters.AcceptGZip() without losing the non-blocking nature of that client.

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

No branches or pull requests