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

Add support for batching requests across unrelated fetches #504

Conversation

jordiolivares
Copy link
Contributor

I noticed that when processing Fetch requests across Fetch.run calls on an HTTP server the fetch requests weren't batched across executions. This means that if we were processing for example 3000 requests simultaneously where each Fetch is for a single ID, we would have 3000 calls to the DataSource.fetch method.

This PR thus adds support for bundling the calls within a given interval in order to produce a single DataSource.batch call instead. This would massively help throughput of HTTP servers at the cost of latency

@jordiolivares
Copy link
Contributor Author

For some reason the check fails due to an HTTP error even before the code is compiled/tested with this:

HttpError: Resource not accessible by integration

I have no clue why this is failing

@juanpedromoreno
Copy link
Contributor

Thanks so much for your time @jordiolivares .

I've checked locally, and after running sbt ci-test, I get these issues:

[warn] /temp/fetch/fetch/src/main/scala/datasource.scala isn't formatted properly!
[warn] /temp/fetch/fetch/src/main/scala/datasource.scala isn't formatted properly!
[error] 1 files must be formatted
[error] 1 files must be formatted

Running scalafmt, and running ci-test again, I get the following compilation issues:

[error] /Users/juanpedromoreno/temp/fetch/fetch/src/main/scala/datasource.scala:123:32: value groupMap is not a member of List[(I, Callback)]
[error]           val asMap        = x.groupMap(_._1)(_._2)
[error]                                ^
[error] /Users/juanpedromoreno/temp/fetch/fetch/src/main/scala/datasource.scala:123:32: value groupMap is not a member of List[(I, Callback)]
[error]           val asMap        = x.groupMap(_._1)(_._2)
[error]                                ^
https://repo1.maven.org/maven2/org/http4s/http4s-blaze-client_2.12/0.23.3/http4s-blaze-client_2.12-0.23.3.pom
  100.0% [##########] 5.3 KiB (35.9 KiB / s)
[error] /Users/juanpedromoreno/temp/fetch/fetch/src/main/scala/datasource.scala:130:41: type mismatch;
[error]  found   : F[Nothing]
[error]  required: F[A]
[error] Note: Nothing <: A, but type F is invariant in type _.
[error] You may wish to define _ as +_ instead. (SLS 4.5)
[error]           val fiberWork = F.handleError(resultsHaveBeenSent) { ex =>
[error]                                         ^
[error] /Users/juanpedromoreno/temp/fetch/fetch/src/main/scala/datasource.scala:130:41: type mismatch;
[error]  found   : F[Nothing]
[error]  required: F[A]
[error] Note: Nothing <: A, but type F is invariant in type _.
[error] You may wish to define _ as +_ instead. (SLS 4.5)
[error]           val fiberWork = F.handleError(resultsHaveBeenSent) { ex =>
[error]                                         ^

Could you please take a second look? Thanks!

@juanpedromoreno
Copy link
Contributor

In the meantime, I'm taking a look at the CI/CD issue with the PR labeler.

@jordiolivares
Copy link
Contributor Author

jordiolivares commented Sep 14, 2021

Seems like I've been in Scala 2.13 for so long I had forgotten about Scala 2.12. I've made the compilation fix, so feel free to review it again!

@juanpedromoreno
Copy link
Contributor

Thank so much @jordiolivares ! Great contribution 🙌

@juanpedromoreno juanpedromoreno merged commit c96f74a into xebia-functional:main Sep 14, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants