martin-g commented on code in PR #1698:
URL:
https://github.com/apache/datafusion-ballista/pull/1698#discussion_r3236845893
##########
ballista/scheduler/src/state/executor_manager.rs:
##########
@@ -114,7 +114,7 @@ impl ExecutorManager {
}
let alive_executors = self.get_alive_executors();
if alive_executors.is_empty() {
- debug!("There's no alive executors for binding tasks");
+ warn!("There's no alive executors for binding tasks");
Review Comment:
```suggestion
warn!("There are no alive executors to bind tasks");
```
##########
ballista/scheduler/src/scheduler_server/query_stage_scheduler.rs:
##########
@@ -88,6 +88,10 @@ impl<T: 'static + AsLogicalPlan, U: 'static +
AsExecutionPlan>
time_recorder = Some((Instant::now(), event.clone()));
};
let event_sender = EventSender::new(tx_event.clone());
+ self.metrics_collector.set_pending_tasks_queue_size(
+ self.state.task_manager.pending_task_number().await as u64,
Review Comment:
Should this be updated here ?
It looks expensive to iterate the jobs on every event.
Also, the metrics should be updated **after** processing the event.
##########
ballista/scheduler/src/state/task_manager.rs:
##########
@@ -270,6 +270,16 @@ impl<T: 'static + AsLogicalPlan, U: 'static +
AsExecutionPlan> TaskManager<T, U>
self.active_job_cache.len()
}
+ /// Get the total number of pending tasks across all active jobs.
+ pub async fn pending_task_number(&self) -> usize {
+ let mut count = 0;
+ for pair in self.active_job_cache.iter() {
+ let graph = pair.value().execution_graph.read().await;
Review Comment:
DashMap::iter() acquires a Read lock on the map.
The lock is kept here while `.await`-ing and this prevents any writers to
proceed.
```suggestion
let execution_graph = pair.value().execution_graph.clone();
let graph = execution_graph.read().await;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
