Hi all,
I'd like to restart the discussion on performance testing. This time
it's about a layer that AIP-59 doesn't cover: micro-benchmarks for
single hot-path methods, run automatically on pull requests.
WHY NOW
While addressing review comments on Task Iteration (#62922 [1]), a fix
to CommsDecoder.asend changed it from two thread hops per request to
one. Every async SDK call a task makes goes through it: Variables,
Connections, XComs, the task state store. Measured locally (median of
three runs, before -> after):
send, one sync round trip (code unchanged) 11.5 us -> 11.4 us (none)
asend, one async round trip 108.7 us -> 58.0 us (-47%)
asend, 50 concurrent coroutines on one loop 18.6 ms -> 11.7 ms (-37%)
The reverse change, an extra thread hop added in passing, would have
cost the same amount, and nothing in CI would have noticed. Today we
only find such regressions when someone measures by hand, often after
a release: #71721 [2] reports structured task logging in 3.2.0
multiplying high-volume task runtime by 2.64.
HOW THIS RELATES TO AIP-59
AIP-59 (Performance tests framework [3]) measures the footprint of
whole components: scheduler, workers, triggerer, database.
Micro-benchmarks would sit one layer below it. They target single
methods on hot paths and flag a regression in the PR that introduces
it.
A DRAFT TO MAKE IT CONCRETE
Draft PR #74562 [4] adds:
*
task-sdk/benchmarks/, next to tests/ and outside the default test
*
paths, with three benchmarks of the supervisor comms written as
plain pytest functions using pytest-codspeed (MIT);
*
a "benchmarks" dependency group;
*
a workflow that runs the benchmarks through CodSpeed in simulation
*
That mode counts CPU instructions rather than measuring time,
so results don't depend on how busy the shared runner is, and
CodSpeed comments each PR with its change against main.
CodSpeedHQ/action is already on the ASF allow-list (v5.2.1, pinned in
apache/infrastructure-actions). It authenticates through OIDC, so no
secret is needed. Someone with admin rights would still have to
enable the CodSpeed app for apache/airflow.
QUESTIONS I'D LIKE YOUR VIEWS ON
1.
Do we want micro-benchmarks for hot paths at all? If so, is this
*
an extension of AIP-59, or does it need its own AIP?
2.
Where should they live and how should they be named? I'd propose
*
<distribution>/benchmarks/, with paths mirroring the source module
*
they cover:
src/airflow/sdk/execution_time/comms.py
-> benchmarks/execution_time/test_comms_benchmark.py
CI could then run only the benchmarks covering the files a PR
changes.
1.
Which tool? pytest-codspeed with the hosted comparison, or
*
something purely local like pytest-benchmark? The local option
*
compares against a saved baseline with a threshold, but wall-time
*
thresholds on shared runners are noisy.
2.
What should a regression do: report only (a PR comment), or fail
*
the check above a threshold? I would personally for failure.
3.
Which paths should come first? Beyond the comms, candidates are
*
XCom serialisation, template rendering, DAG parsing, and the
*
scheduler's critical-section queries. We could add it when doing
changes or fixes to the code in upcoming PR's.
[1] https://github.com/apache/airflow/pull/62922
[2] https://github.com/apache/airflow/issues/71721
[3]
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-59+Performance+tests+framework
[4] https://github.com/apache/airflow/pull/74562
Thanks,
David
General (Internal Property)
