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

Larger play thread pool, async fossil client #7139

Merged
merged 20 commits into from
Jun 26, 2023
Merged

Larger play thread pool, async fossil client #7139

merged 20 commits into from
Jun 26, 2023

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Jun 14, 2023

  • Remove usages of ExecutionContext.global. Using that discouraged because there are no limits to how many threads it spawns. Before this PR, we randomly used it whereever it was too hard to inject the play execution context.
  • Adds a sensible config for the play execution context. Before this PR, the default config was used, which yielded way too few threads for the amount of blocking operations we have.
  • Adapts most of the fossildb gprc client to async and adapts its error handling accordingly. With one exception, which is getMultipleKeys, because that is used in Iterators, which are harder to rewrite to async (this may be a future optimization)

TODO

  • get rid of global execution contest
  • adapt akka config for some more threads
    • also in datastore + tracingstore
  • go for more async io usages
    • async fossil client
      • adapt health check
      • adapt get request
      • adapt other requests
      • error handling (exceptions are thrown inside the future?)

URL of deployed dev instance (used for testing):

Steps to test:

  • Test that data loading for remote datasets still works
  • Test that annotating (save, reload, download) still works
  • To actually test the improvements in parallelism, I used the test route added in 4c96ce4 – sending a lot of parallel requests to /tracings/test showed in the logging how the requests were answered in parallel

Issues:


  • Removed dev-only changes like prints and application.conf edits
  • Considered common edge cases
  • Needs datastore update after deployment

@fm3 fm3 changed the title WIP: Control thread pools, async fossil client Larger play thread pool, async fossil client Jun 15, 2023

import com.scalableminds.util.tools.Fox

import scala.concurrent.ExecutionContext.Implicits.global
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole file was unused

@@ -20,4 +20,4 @@ addSbtPlugin("com.sksamuel.scapegoat" %% "sbt-scapegoat" % "1.1.1")
addSbtPlugin("net.vonbuchholtz" % "sbt-dependency-check" % "3.1.3")

//protocol buffers
libraryDependencies += "com.thesamet.scalapb" %% "compilerplugin" % "0.11.12"
libraryDependencies += "com.thesamet.scalapb" %% "compilerplugin" % "0.11.13"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrade not really needed for this, but shouldn’t hurt (I added it while debugging something which turned out to be caused elsewhere)

@fm3 fm3 marked this pull request as ready for review June 19, 2023 09:56
@fm3 fm3 requested a review from frcroth June 19, 2023 09:59
CHANGELOG.unreleased.md Outdated Show resolved Hide resolved
test/backend/NMLUnitTestSuite.scala Show resolved Hide resolved
@fm3 fm3 requested a review from frcroth June 22, 2023 07:45
@fm3 fm3 enabled auto-merge (squash) June 26, 2023 08:01
@fm3 fm3 merged commit 7b941ca into master Jun 26, 2023
2 checks passed
@fm3 fm3 deleted the thread-pools branch June 26, 2023 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize ThreadPool usage, make more use of async IO
2 participants