Pebble32 opened a new pull request, #72475:
URL: https://github.com/apache/airflow/pull/72475

   Add opt-in, deterministic jitter to every cron-based timetable via 
`CronMixin`: two kw-only params, `seed` and `max_jitter`, on 
`CronTriggerTimetable`, `CronDataIntervalTimetable`, 
`MultipleCronTriggerTimetable` and `CronPartitionTimetable`, in both the Task 
SDK and airflow-core, with serialization wired through. Each DAG's runs are 
shifted by a fixed offset drawn from `[0, max_jitter)`, so DAGs that share a 
cron expression no longer all fire at the same instant.
   
   This is the follow-up @uranusjr suggested on #69705 
(https://github.com/apache/airflow/pull/69705#issuecomment-5232872059): rather 
than a standalone timetable, the jitter lives in `CronMixin` so all cron 
scheduling inherits it. It supersedes #69705, which I will close once this 
lands. cc @kaxil, who reviewed the original.
   
   ### Why
   
   `@daily` expands to `0 0 * * *`, so **every** daily DAG in a deployment is 
scheduled at exactly midnight, and the same holds for any shared cron 
expression. The existing ways to deal with this don't actually spread the 
schedule:
   
   | Existing option | Why it doesn't spread the schedule |
   | --- | --- |
   | Hand-pick a unique minute per DAG | Manual, drifts and re-collides as the 
DAG count grows, and throws away the `@daily` intent |
   | Hash the DAG id into a literal cron string | Same loss of intent; not 
reusable across DAGs/teams; every author re-implements it ad hoc |
   | Pools / concurrency limits (`parallelism`, `max_active_tasks_per_dag`, 
pool slots) | Cap how many tasks run at once, but the runs are still 
*scheduled* at the same instant. They manage contention downstream; jitter 
reduces the peak at the source. The two are complementary, not alternatives. |
   
   Jitter is peak shaving on top of concurrency control, not a replacement for 
it. It is the same idea as Jenkins' `H` cron syntax. The offset is 
deterministic: the same `seed` (e.g. the DAG id) always maps to the same 
offset, so runs stay stable across scheduler restarts and serialization. 
Motivated by the discussion in 
https://github.com/apache/airflow/discussions/69027.
   
   ### What
   
   - `seed`/`max_jitter` added to `CronMixin` in both layers, so all four cron 
timetables inherit them; the concrete classes thread the params through their 
constructors and `serialize`/`deserialize`, and the `encoders.py` variants emit 
them.
   - The offset is `md5(seed) % max_jitter`, computed in integer microseconds 
(FIPS-safe `hashlib_wrapper.md5`), and applied as a "strip → cron → apply" 
coordinate shift in `CronMixin._get_next`/`_get_prev`. Because 
`_align_to_next`/`_align_to_prev` build on those primitives, every cron 
timetable inherits the shift with no per-class scheduling code.
   - **Fully opt-in and safe by default**: with `max_jitter` at its default of 
zero the offset is zero and every timetable behaves *identically* to before. 
Nothing changes for anyone who doesn't use it.
   
   ### Design decisions
   
   - **Uniform shift for data-interval timetables.** The cron boundaries define 
the window, so the whole interval moves by the offset: same length, consecutive 
runs stay contiguous, but it no longer starts exactly on the cron time (00:35 
to 00:35 instead of 00:00 to 00:00). Documented, with guidance to keep 
`max_jitter` small relative to the period.
   - **`MultipleCronTriggerTimetable` children share one offset.** The same 
`seed`/`max_jitter` is passed to every child, so the whole DAG shifts in 
lockstep and its fire times keep their relative spacing.
   - **Serialization only emits `seed`/`max_jitter` when jitter is set.** The 
wire format of existing DAGs is unchanged (so nothing is re-serialized on 
upgrade) and `deserialize` tolerates missing keys. The existing serialization 
tests pass untouched.
   - **Jitter is part of `__eq__`/`__hash__`**, since timetables differing only 
in jitter produce different schedules. While adding this I fixed a latent bug: 
`__hash__` included the pendulum `Timezone` object, which is unhashable, so 
`hash()` on any cron timetable raised; it now hashes `str(timezone)`.
   - **Empty `seed` with `max_jitter > 0` raises** in both layers: it would 
give every DAG the same offset and merely move the herd instead of spreading it.
   
   ### Tests
   
   `airflow-core/tests/unit/timetables/test_cron_timetable_jitter.py` (28 
tests), modeled on the existing `test_trigger_timetable.py`:
   
   - jittered runs equal plain cron runs shifted by the fixed offset, across a 
catchup sequence including a DST spring-forward (`America/New_York`);
   - zero `max_jitter` reproduces the plain timetable exactly (catchup on/off);
   - offsets are deterministic for a given seed, bounded to `[0, max_jitter)`, 
spread across distinct seeds, and correct for sub-second windows;
   - the empty-seed guard fires in both layers for all four timetables;
   - core `serialize`/`deserialize` and SDK encode → decode round-trips 
preserve the offset for all four timetables;
   - data-interval jitter shifts both bounds uniformly and keeps consecutive 
runs contiguous;
   - `MultipleCronTriggerTimetable` children share a single offset;
   - un-jittered timetables serialize without the jitter keys;
   - jitter participates in equality, and equal timetables hash equal.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes (please specify the tool below)
   
   Generated-by: Claude (Claude Code), following [the 
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   
   ---
   
   related: #69705
   related: https://github.com/apache/airflow/discussions/69027
   


-- 
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]

Reply via email to