Philippus / elastic4s

🔍 Elasticsearch Scala Client - Reactive, Non Blocking, Type Safe, HTTP Client
Apache License 2.0
1.63k stars 691 forks source link

Cats Effect 2 executor doesn't shift back to the compute pool #2445

Open pjurczenko opened 3 years ago

pjurczenko commented 3 years ago

The current implementation of Cats Effect 2 executor relies solely on Async.async method, which doesn't shift the execution back to the compute pool. This means that operations subsequent to the ElasticSearch queries will be performed on the dispatcher threads used for the non-blocking I/O, which is definitely undesirable. This is how the executor could look like to prevent this:

import cats.syntax.apply._

class CatsEffectExecutor[F[_]: Async: ContextShift] extends Executor[F] {
  override def exec(client: HttpClient, request: ElasticRequest): F[HttpResponse] =
    Async[F].async(cb => client.send(request, cb)) <* ContextShift[F].shift
}

However, such a change would be breaking the binary compatibility.

@sksamuel how do you approach such breaking changes in the elastic4s releases?

sksamuel commented 2 years ago

We can do this for the 8.0 release which will contain other breaking changes.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-sebastien-boulet commented 2 years ago

This is still relevant

igor-vovk commented 2 weeks ago

I think this issue can be closed since it's not relevant since cats-effect 3

lenguyenthanh commented 3 days ago

👍 for close this