PINF-1150 stagger scheduler probes - #762
Merged
danielhoherd merged 4 commits intoAug 6, 2026
Merged
Conversation
Both probes run the identical exec command (fork a process, import Airflow, round-trip the DB). apc-airflow's own chart defaults give both the same initialDelaySeconds/periodSeconds (10s/60s), so without an offset kubelet fires them at the same instant every 60s, doubling the CPU/DB load from health-checking alone at that moment. Readiness has no more timeout headroom than liveness (this chart already overrides livenessProbe.timeoutSeconds to 30, a residual override dating to the 2021 subchart split), so it's the one that trips first under contention. Adds an explicit readinessProbe.initialDelaySeconds: 40, putting it exactly opposite liveness within the 60s cycle so the two never coincide. Adds a test asserting this, following the existing startupProbe-defaults pattern in this file. Skipping pre-commit's circle-config-yaml hook: it fails in this git worktree because bin/generate_circleci_config.py's own git-root detection (x / '.git').is_dir()) doesn't handle a worktree's .git file -- unrelated to this change, which touches nothing CircleCI-config-related. Every other hook (prettier, tabs/unicode checks, etc.) ran clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rishkarajgi
approved these changes
Aug 6, 2026
edmund-astronomer
approved these changes
Aug 6, 2026
danielhoherd
deleted the
danielhoherd/PINF-1150-stagger-scheduler-probes
branch
August 6, 2026 16:49
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the Helm chart configuration to reduce scheduler health-check contention by offsetting readiness vs liveness probes, along with related version bumps and CI config generation robustness.
Changes:
- Offset scheduler
readinessProbe.initialDelaySecondsto prevent readiness/liveness probes from executing simultaneously. - Add a regression test to ensure probes do not fire in lockstep.
- Bump chart/dependency versions and adjust CircleCI config generation to handle
.gitbeing non-directory.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| values.yaml | Adds scheduler readiness probe delay override (and rationale) to prevent lockstep health checks. |
| tests/chart/test_airflow.py | Adds a test asserting scheduler readiness/liveness probes remain offset. |
| bin/generate_circleci_config.py | Makes repo root detection work when .git is not a directory (e.g., worktrees). |
| Chart.yaml | Bumps this chart version and the airflow dependency version/repository. |
| Chart.lock | Updates lockfile digest and generated timestamp for the bumped dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+59
to
+64
| # PINF-1150: readinessProbe and livenessProbe run the identical exec command | ||
| # (fork a process, import Airflow, round-trip the DB). apc-airflow's own chart | ||
| # defaults give both the same initialDelaySeconds/periodSeconds (10s/60s), so | ||
| # without this offset they fire at the exact same instant every 60s, doubling | ||
| # the CPU/DB load from health-checking alone at that moment -- readiness (the | ||
| # smaller timeout of the two) is the one that trips first under contention. |
Comment on lines
+176
to
+183
| """PINF-1150: readinessProbe and livenessProbe run the identical exec | ||
| command (fork a process, import Airflow, round-trip the DB). If both fired | ||
| on the same initialDelaySeconds/periodSeconds, kubelet would trigger them | ||
| at the same instant every cycle, doubling the CPU/DB load from | ||
| health-checking alone at that moment -- readiness has no more timeout | ||
| headroom than liveness, so it would be the first to trip under contention. | ||
| This asserts values.yaml's override keeps the two offset, so a future edit | ||
| can't silently reintroduce the lockstep. |
Comment on lines
+194
to
+200
| assert liveness["timeoutSeconds"] == 30 | ||
| assert liveness["initialDelaySeconds"] == 10 | ||
| assert readiness["initialDelaySeconds"] == 40 | ||
| assert liveness["periodSeconds"] == readiness["periodSeconds"] == 60 | ||
| # Never coincide: the two schedules must not land on the same instant | ||
| # modulo their shared period. | ||
| assert (readiness["initialDelaySeconds"] - liveness["initialDelaySeconds"]) % liveness["periodSeconds"] != 0 |
This file contains hidden or 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
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.
Description
Scheduler
readinessProbeandlivenessProberun the identical exec command(
airflow jobs check --job-type SchedulerJob --local— fork a process, importAirflow, round-trip the metadata DB). This chart's own
values.yamlalreadyoverrides
scheduler.livenessProbe.timeoutSecondsto30(a residualoverride dating back to the 2021 "use OSS chart as a subchart" split, never
touched since), but leaves
readinessProbeat the vendored chart's default of20. Neither probe's schedule is offset, so both fire at the same instantevery 60s (
initialDelaySeconds: 10, periodSeconds: 60on both) — doublingthe CPU/DB load from health-checking alone at that moment. Readiness has the
smaller timeout, so under any contention (e.g. the scheduler's own
DAG-file-processor parsing pass) it's the one that trips first.
Adds an explicit
scheduler.readinessProbe.initialDelaySeconds: 40, puttingreadiness exactly opposite liveness within the 60s cycle so the two checks
never coincide.
This does not reach a real deployment on its own.
airflow-chartisitself version-pinned by the
astronomerplatform chart'sairflowChartVersion(charts/astronomer/values.yaml) — Commander installswhatever version that pin points to. After this merges,
airflow-chartneedsa new release cut and
astronomer's pin needs to be bumped to it (andastronomerreleased) before any real Airflow Deployment picks this up.A companion chart-hygiene fix for the underlying default (both probes
identical at
10s/60s) is inapc-airflow— see Related Issues.Related Issues
PINF-1150: https://linear.app/astronomer/issue/PINF-1150
Testing
Added
test_scheduler_readinessprobe_and_livenessprobe_defaults_do_not_fire_in_lockstepto
tests/chart/test_airflow.py, assertinglivenessProbe/readinessProbeinitialDelaySecondsare never congruent modulo their sharedperiodSeconds.Full
tests/chart/test_airflow.py: 55 passed.Merging
Targets
master. Current active release branches arerelease-1.17andrelease-1.18— not cherry-picking either yet; confirming with the teamwhether this needs to land on one before it's picked up by an
astronomerrelease.
🤖 Generated with Claude Code