Andrushika commented on code in PR #73316:
URL: https://github.com/apache/airflow/pull/73316#discussion_r4046182566
##########
scripts/ci/analyze_ci_job_durations.py:
##########
@@ -91,6 +94,13 @@
)
IMAGE_WORK_LABEL = "Image build, pull & push"
+# Jobs that swing on events outside the repository, where a single elevated
run is not a
+# regression. `constraints-version-check` re-resolves every provider a release
wave has just
+# published, so these jobs double for a few days after each wave and fall back
once the
+# constraints catch up. Alerting on that sawtooth teaches readers to ignore
the alert, so they
+# must stay elevated for SPIKY_JOB_PERSISTENCE_DAYS first — the rule the image
build follows.
+SPIKY_JOB_NAME_PATTERNS = ("*Deps *:constraints*",)
Review Comment:
The trailing `*` also matches `Deps 3.11:constraints-no-providers`, but that
mode does not install providers, so a release wave should not move it. I pulled
the wall-clock times of the three modes for the scheduled runs from 09-04 to
09-16:
| date | constraints | source-providers | no-providers |
|---|---|---|---|
| 09-12 | 916 | 1182 | 435 |
| 09-13 | 809 | 1242 | 510 |
| 09-14 | 1143 | 1565 | 446 |
| 09-15 | 1699 | 1463 | 533 |
| 09-16 | 1541 | 1698 | 577 |
The first two jump after the 09-14 wave, the third stays flat. With the
current pattern a real slowdown in `no-providers` would wait three days for no
reason. Would `("*Deps *:constraints", "*Deps *:constraints-source-providers")`
make sense here?
##########
scripts/ci/analyze_ci_job_durations.py:
##########
Review Comment:
I think taking the baseline from the oldest five runs can pick up the
previous wave. The window is 25 scheduled runs, about a month, and the provider
waves come every one or two weeks with two or three elevated runs each, so the
oldest five will sometimes hold most of a wave.
For example, with the `WAVE` numbers from the new test as the oldest five
runs `(1203, 1257, 855, 425, 577)`, the baseline is 855 and the threshold 1069.
A job that then settles at 900 (from a real 384 before) never flags, and
nothing tells us the threshold was inflated. For the image build this was fine
because a cache miss is one run, but this job stays up for several runs.
Would it make sense to take the median of everything outside the streak
instead? Something like computing the streak first against a provisional
threshold, then `baseline = median(seconds for _, seconds in
per_run[len(streak):])`. Twenty-odd runs are hard to pull up with two or three
high ones. Happy to push a test for it if useful.
Drafted-by: Claude Code (Fable 5.1)
--
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]
