Separate cpu-bound (query-execution) and IO-bound(heartbeat) to … #1883
Conversation
…diff tokio runtime in ballista_executor
|
retest this please |
There was a problem hiding this comment.
I feel strange reviewing / approving this PR (as I wrote a bunch of the code originally) but its use makes sense to me (obviously :) ).
@houqp or @thinkharderdev or @realno do you have any additional comments or thoughts on the approach?
| } | ||
|
|
||
| /// signals shutdown of this executor and any Clones | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
I wonder if this #allow is really necessary?
There was a problem hiding this comment.
For fix clippy, In the future if realized close the executor, will delete this.
| /// The worker thread priority is set to low so that such tasks do | ||
| /// not starve other more important tasks (such as answering health checks) | ||
| /// | ||
| pub fn new(thread_name: impl Into<String>, num_threads: usize) -> Self { |
There was a problem hiding this comment.
FWIW I think @Darksonn and others have noted that a tokio Handle might also be able to be used here https://docs.rs/tokio/1.17.0/tokio/runtime/struct.Handle.html
| @@ -261,6 +262,8 @@ impl<T: 'static + AsLogicalPlan, U: 'static + AsExecutionPlan> TaskRunnerPool<T, | |||
| let executor_server = self.executor_server.clone(); | |||
| tokio::spawn(async move { | |||
| info!("Starting the task runner pool"); | |||
| //TODO make it configurable | |||
| let dedicated_executor = DedicatedExecutor::new("task_runner", 4); | |||
There was a problem hiding this comment.
I think this should default to the number of available CPU cores.
There was a problem hiding this comment.
set default to 4 due to: in ballista conf concurrent_tasks set 4 . We will make it configurable
Co-authored-by: Andrew Lamb <[email protected]>
|
retest this plz. |
|
I'll merge this one in to keep things going and we can adjust the default number of cores as follow on work |
…diff tokio runtime in ballista_executor
Closes #1770.
Rationale for this change
Have tested in my local
one scheduler + one executor
What changes are included in this PR?
No
Are there any user-facing changes?