Change with_tokio_rt to accept Arc<Runtime>. #599
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows to share tokio runtimes across different sub-systems inside your application. Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
gz
added a commit
to feldera/feldera
that referenced
this pull request
Sep 5, 2024
If an adhoc query is sent it will be handled in the same tokio runtime that is running the webserver. This raises the questions of how many cores the runtime should have. Datafusion will need one task for each partition we're reading from. And we have as many partitions as dbsp workers. So maybe we should've just change HTTP_NUM_WORKERS from 4 to `config.global.runtime_workers`. Except, this doesn't quite help because if your query is very expensive like `select * from table_with_100m`, all it's task run for quite some time and starve out the HTTP request. So now the UI no longer updates. Ok, so maybe it should just be `workers*2`, but if workers is set near the amount of CPU cores on the machine then we'll overprovision the tokio runtime and that will slow things down too. So for now we just set this to #cores on the machine by not configuring it (this is the default). Note aside, ideally we can just swap out the actix runtime with `dbsp::runtime::TOKIO` which is what we use everywhere else and this already uses the #cores in the runtime to run the tasks. However, this also doesn't work because the actix API has a stupid bug in the interface. I submitted a PR to fix this here: actix/actix-net#599 So for now, we'll just have two tokio runtimes, with Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
gz
added a commit
to feldera/feldera
that referenced
this pull request
Sep 7, 2024
If an adhoc query is sent it will be handled in the same tokio runtime that is running the webserver. This raises the questions of how many cores the runtime should have. Datafusion will need one task for each partition we're reading from. And we have as many partitions as dbsp workers. So maybe we should've just change HTTP_NUM_WORKERS from 4 to `config.global.runtime_workers`. Except, this doesn't quite help because if your query is very expensive like `select * from table_with_100m`, all it's task run for quite some time and starve out the HTTP request. So now the UI no longer updates. Ok, so maybe it should just be `workers*2`, but if workers is set near the amount of CPU cores on the machine then we'll overprovision the tokio runtime and that will slow things down too. So for now we just set this to #cores on the machine by not configuring it (this is the default). Note aside, ideally we can just swap out the actix runtime with `dbsp::runtime::TOKIO` which is what we use everywhere else and this already uses the #cores in the runtime to run the tasks. However, this also doesn't work because the actix API has a stupid bug in the interface. I submitted a PR to fix this here: actix/actix-net#599 So for now, we'll just have two tokio runtimes, with Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
gz
added a commit
to feldera/feldera
that referenced
this pull request
Sep 7, 2024
If an adhoc query is sent it will be handled in the same tokio runtime that is running the webserver. This raises the questions of how many cores the runtime should have. Datafusion will need one task for each partition we're reading from. And we have as many partitions as dbsp workers. So maybe we should've just change HTTP_NUM_WORKERS from 4 to `config.global.runtime_workers`. Except, this doesn't quite help because if your query is very expensive like `select * from table_with_100m`, all it's task run for quite some time and starve out the HTTP request. So now the UI no longer updates. Ok, so maybe it should just be `workers*2`, but if workers is set near the amount of CPU cores on the machine then we'll overprovision the tokio runtime and that will slow things down too. So for now we just set this to #cores on the machine by not configuring it (this is the default). Note aside, ideally we can just swap out the actix runtime with `dbsp::runtime::TOKIO` which is what we use everywhere else and this already uses the #cores in the runtime to run the tasks. However, this also doesn't work because the actix API has a stupid bug in the interface. I submitted a PR to fix this here: actix/actix-net#599 So for now, we'll just have two tokio runtimes, with Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
gz
added a commit
to feldera/feldera
that referenced
this pull request
Sep 7, 2024
If an adhoc query is sent it will be handled in the same tokio runtime that is running the webserver. This raises the questions of how many cores the runtime should have. Datafusion will need one task for each partition we're reading from. And we have as many partitions as dbsp workers. So maybe we should've just change HTTP_NUM_WORKERS from 4 to `config.global.runtime_workers`. Except, this doesn't quite help because if your query is very expensive like `select * from table_with_100m`, all it's task run for quite some time and starve out the HTTP request. So now the UI no longer updates. Ok, so maybe it should just be `workers*2`, but if workers is set near the amount of CPU cores on the machine then we'll overprovision the tokio runtime and that will slow things down too. So for now we just set this to #cores on the machine by not configuring it (this is the default). Note aside, ideally we can just swap out the actix runtime with `dbsp::runtime::TOKIO` which is what we use everywhere else and this already uses the #cores in the runtime to run the tasks. However, this also doesn't work because the actix API has a stupid bug in the interface. I submitted a PR to fix this here: actix/actix-net#599 So for now, we'll just have two tokio runtimes, with Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
|
To avoid breaks we could add a new method for this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows to share tokio runtimes across different sub-systems inside your application.
PR Type
Feature, Refactor
#580
PR Checklist
Check your PR fulfills the following:
Overview
The current signature of https://docs.rs/actix-rt/latest/actix_rt/struct.System.html#method.with_tokio_rt expects a
Runtime. It would be better if I can supply an Arc instead. The reason being that is that in a bigger application we have multiple systems trying to leverage tokio. With the given API I still need to create a separate runtime for actix. Having the same runtime is helpful for better control over CPU resources as there aren'tnruntimes competing for CPU time.FWIW this is a breaking change so if there alternative ways to achieve this I'm happy to give those a try too.