You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "akshaychitneni (via GitHub)" <gi...@apache.org> on 2026/08/10 18:16:26 UTC
[PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
akshaychitneni opened a new pull request, #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279
Which issue does this PR close?
Follow-up to #2029 / #2212, addressing the timeout interaction surfaced by #2240 (https://github.com/apache/datafusion-ballista/pull/2240#discussion_r3732320008)
Rationale for this change
The empty-cluster grace timer defaulted to 30s, but the executor heartbeat interval defaults to 60s. Since #2240 routes task-launch-failure removals through the same ExecutorLost path (aggressive removal that also drops the heartbeat), a healthy executor removed by a transient blip re-registers only on its next heartbeat (~≤60s) — which the 30s timer could beat,
spuriously failing the job.
What changes are included in this PR?
no_executors_grace_period_seconds now defaults to executor_timeout_seconds (guaranteed longer than the heartbeat interval; the scheduler can't read the executor's heartbeat setting directly).
CLI arg is Option, resolving to executor_timeout_seconds when unset; SchedulerConfig::Default matches.
Removal stays immediate (fast rescheduling); only the fail decision waits, and the timer's re-check still cancels the failure if the executor re-registers. Explicit values (incl. 0 = fail-fast) honored.
Adds config tests (default resolution + grace >= executor_timeout invariant).
Are there user-facing changes?
Yes — the default no-executors grace period changes from 30s to executor_timeout_seconds (180s by default); --no-executors-grace-period-seconds is now optional and defaults to that.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on code in PR #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279#discussion_r3756273231
##########
ballista/scheduler/src/config.rs:
##########
@@ -667,7 +662,12 @@ impl TryFrom<Config> for SchedulerConfig {
executor_timeout_seconds: opt.executor_timeout_seconds,
expire_dead_executor_interval_seconds: opt
.expire_dead_executor_interval_seconds,
- no_executors_grace_period_seconds: opt.no_executors_grace_period_seconds,
+ // Default to the executor-liveness timeout when unset, so the grace
+ // is always >= the executor heartbeat interval and a transiently
+ // removed but healthy executor can re-register before it elapses.
+ no_executors_grace_period_seconds: opt
+ .no_executors_grace_period_seconds
+ .unwrap_or(opt.executor_timeout_seconds),
Review Comment:
Should it be multiple of value, just to avoid race ?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm merged PR #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on code in PR #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279#discussion_r3761888030
##########
ballista/scheduler/src/config.rs:
##########
@@ -667,7 +662,12 @@ impl TryFrom<Config> for SchedulerConfig {
executor_timeout_seconds: opt.executor_timeout_seconds,
expire_dead_executor_interval_seconds: opt
.expire_dead_executor_interval_seconds,
- no_executors_grace_period_seconds: opt.no_executors_grace_period_seconds,
+ // Default to the executor-liveness timeout when unset, so the grace
+ // is always >= the executor heartbeat interval and a transiently
+ // removed but healthy executor can re-register before it elapses.
+ no_executors_grace_period_seconds: opt
+ .no_executors_grace_period_seconds
+ .unwrap_or(opt.executor_timeout_seconds),
Review Comment:
Sorry I've misread property used, apologies
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
Posted by "akshaychitneni (via GitHub)" <gi...@apache.org>.
akshaychitneni commented on code in PR #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279#discussion_r3761627730
##########
ballista/scheduler/src/config.rs:
##########
@@ -667,7 +662,12 @@ impl TryFrom<Config> for SchedulerConfig {
executor_timeout_seconds: opt.executor_timeout_seconds,
expire_dead_executor_interval_seconds: opt
.expire_dead_executor_interval_seconds,
- no_executors_grace_period_seconds: opt.no_executors_grace_period_seconds,
+ // Default to the executor-liveness timeout when unset, so the grace
+ // is always >= the executor heartbeat interval and a transiently
+ // removed but healthy executor can re-register before it elapses.
+ no_executors_grace_period_seconds: opt
+ .no_executors_grace_period_seconds
+ .unwrap_or(opt.executor_timeout_seconds),
Review Comment:
Kept the default equal to executor_timeout_seconds rather than a multiple: 180s is already ~3× the 60s heartbeat. Increasing further might push total-loss failures to 6+ min
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
Posted by "akshaychitneni (via GitHub)" <gi...@apache.org>.
akshaychitneni commented on PR #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279#issuecomment-5256999015
> Thanks @akshaychitneni this makes sense, one minor comment, and a question if we can add validation method now when we have dependencies between configuration values
Thanks. Added SchedulerConfig::validate(), called from start_server. It warns when no_executors_grace_period_seconds (nonzero) is below executor_timeout_seconds, and when the timeout isn't greater than the expire interval
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
Posted by "akshaychitneni (via GitHub)" <gi...@apache.org>.
akshaychitneni commented on code in PR #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279#discussion_r3761627730
##########
ballista/scheduler/src/config.rs:
##########
@@ -667,7 +662,12 @@ impl TryFrom<Config> for SchedulerConfig {
executor_timeout_seconds: opt.executor_timeout_seconds,
expire_dead_executor_interval_seconds: opt
.expire_dead_executor_interval_seconds,
- no_executors_grace_period_seconds: opt.no_executors_grace_period_seconds,
+ // Default to the executor-liveness timeout when unset, so the grace
+ // is always >= the executor heartbeat interval and a transiently
+ // removed but healthy executor can re-register before it elapses.
+ no_executors_grace_period_seconds: opt
+ .no_executors_grace_period_seconds
+ .unwrap_or(opt.executor_timeout_seconds),
Review Comment:
Kept the default equal to executor_timeout_seconds rather than a multiple: 180s is already ~3× the 60s heartbeat. Increasing further might push total-loss failures to 6+ min with multiples
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on code in PR #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279#discussion_r3756058801
##########
ballista/scheduler/src/config.rs:
##########
@@ -399,7 +399,9 @@ impl Default for SchedulerConfig {
grpc_client_max_message_size: 16777216,
executor_timeout_seconds: 180,
expire_dead_executor_interval_seconds: 15,
- no_executors_grace_period_seconds: 30,
+ // Defaults to `executor_timeout_seconds` so the grace is always >=
+ // the executor heartbeat interval (see the field doc).
+ no_executors_grace_period_seconds: 180,
Review Comment:
Should this be 180 by default, if the `executor_heartbeat_interval_seconds` is 60 secs by default?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): default no-executors grace to executor timeout (>= heartbeat) [datafusion-ballista]
Posted by "akshaychitneni (via GitHub)" <gi...@apache.org>.
akshaychitneni commented on code in PR #2279:
URL: https://github.com/apache/datafusion-ballista/pull/2279#discussion_r3761616159
##########
ballista/scheduler/src/config.rs:
##########
@@ -399,7 +399,9 @@ impl Default for SchedulerConfig {
grpc_client_max_message_size: 16777216,
executor_timeout_seconds: 180,
expire_dead_executor_interval_seconds: 15,
- no_executors_grace_period_seconds: 30,
+ // Defaults to `executor_timeout_seconds` so the grace is always >=
+ // the executor heartbeat interval (see the field doc).
+ no_executors_grace_period_seconds: 180,
Review Comment:
Yes. 180 is the default executor_timeout_seconds (the liveness window) giving 3× margin for a transiently-removed executor to re-register before its jobs fail
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org