houqp commented on a change in pull request #1983:
URL: https://github.com/apache/arrow-datafusion/pull/1983#discussion_r824416389
##########
File path: ballista/rust/scheduler/src/scheduler_server/event_loop.rs
##########
@@ -50,32 +47,53 @@ impl<T: 'static + AsLogicalPlan, U: 'static +
AsExecutionPlan>
pub fn new(
state: Arc<SchedulerState<T, U>>,
executors_client: ExecutorsClient,
+ is_test: bool,
) -> Self {
Self {
state,
executors_client,
+ is_test,
}
}
- async fn offer_resources(
- &self,
- job_id: String,
- ) -> Result<Option<SchedulerServerEvent>> {
- let mut available_executors =
self.state.get_available_executors_data();
+ async fn offer_resources(&self, n: u32) ->
Result<Option<SchedulerServerEvent>> {
+ let mut available_executors = if self.is_test {
Review comment:
IMO, setting flags specific for tests in application logic is usually
not an anti-pattern. It makes the code harder to read and adds unnecessary
runtime overhead. If I understand it correctly, the main problem is we don't
have executor heartbeats in tests causing `get_alive_executors` to return an
empty set? If so, I would recommend adding a helper method to set artificial
executor heartbeats directly from tests so we can keep the core logic agnostics
to tests.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
