Speed up breeze start by caching Python bytecode in a docker volume - #72567
Conversation
potiuk
left a comment
There was a problem hiding this comment.
We need different volume per Python version
|
And rebase is needed now. |
3c9afe0 to
64e95be
Compare
|
Rebased. Let's wait for all CI passes. |
|
Unrelated CI fails, seems it could be addressed by #72701. Let's wait for that PR merge and rerun checks. |
… a docker volume Breeze sets PYTHONDONTWRITEBYTECODE=true for its containers, so every airflow process recompiles all mounted sources on import, reading them through the bind mount. On macOS this costs about a second per airflow CLI call and several seconds per component start. Writing .pyc files next to the sources would pollute the host checkout with root-owned files, which is why they were disabled. Pointing PYTHONPYCACHEPREFIX at an external docker volume keeps the cache out of the checkout and lets it survive between container runs. Only shell and start-airflow mount the volume, other containers keep the old behaviour.
64e95be to
73ee29b
Compare
|
Hi @potiuk all CI passed, I think we may merge this. Thanks! |
potiuk
left a comment
There was a problem hiding this comment.
LGTM — my earlier objection about needing a separate volume per Python version doesn't hold, and I checked rather than taking it on trust. Nice change: careful, well-benchmarked, and it keeps CI behaviour untouched.
On the per-Python-version volume
You were right about PEP 3147, and it's worth recording why rather than just resolving the thread. The interpreter tag stays in the filename when PYTHONPYCACHEPREFIX is set — the prefix only relocates the directory tree, it doesn't replace the __pycache__-style naming:
$ PYTHONPYCACHEPREFIX=/tmp/cache python3 -c "import sys; sys.path.insert(0,'src'); import mod_under_test"
$ find /tmp/cache -name '*.pyc'
/tmp/cache/tmp/pycachetest/src/mod_under_test.cpython-313.pycSo a 3.10 and a 3.12 container writing into the same volume produce foo.cpython-310.pyc and foo.cpython-312.pyc side by side and can never read each other's bytecode. One shared volume is correct. Thanks for pushing back with the reasoning instead of just splitting it.
The other things I checked
PYTHONPYCACHEPREFIXactually reaches the container.base.ymlforwards onlyPYTHONWARNINGSexplicitly, everything else rides onenv_file: _generated_docker_compose.env— so the new key arrives. Good that you added it to the generated env rather than to the explicit list.- Empty string really does mean "unset". Both
PYTHONDONTWRITEBYTECODEandPYTHONPYCACHEPREFIXgo through CPython's_Py_GetEnv(), which returnsNULLfor a set-but-empty variable — so always emitting both keys is safe and your comment on that is accurate. That was the bit I most expected to be subtly wrong; it isn't. start-airflowcreates the volume too. It routes throughenter_shell(), socreate_pycache_volume_if_needed()runs before compose seesexternal: true. No missing-volume failure on that path.- The dropped
PYTHONDONTWRITEBYTECODE = Trueconstant inglobal_constants.pyhad no remaining readers — thecheck_docker_resources()occurrence is a separate hardcoded string on a one-offdocker runwith no source mount, so it's unaffected. Good cleanup.
One doc suggestion (non-blocking)
The note reads well, but two facts that reviewers will otherwise re-derive are missing — the Python-version safety above, and that the volume is shared across worktrees. Suggestion inline. On the second point: because breeze always mounts sources at /opt/airflow, two worktrees map onto the same cache paths, and since invalidation is source mtime + size, alternating between worktrees will keep invalidating each other's entries. Correct, just not free — worth a reader knowing before they wonder why the cache feels cold.
No newsfragment needed here and none added — dev tooling isn't user-facing, so that's right.
This review was drafted by an AI-assisted tool and
confirmed by an Airflow maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.More on how Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Backport failed to create: v3-3-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker 8843de3 v3-3-testThis should apply the commit to the v3-3-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
Why
Breeze sets
PYTHONDONTWRITEBYTECODE=truein its containers, so everyairflowprocess recompiles all mounted sources on import, through the bind mount. On macOS this is about 1s per CLI call and several seconds per component start. Writing.pycnext to the sources was disabled on purpose, it would leave root-owned files in the checkout.What
Set
PYTHONPYCACHEPREFIX=/root/.cache/airflow-pycacheand mount an external docker volumeairflow-pycache-volumethere, same pattern asmypy-cache-volume. Onlybreeze shellandbreeze start-airflowmount it and enable the cache. Other containers (breeze testing, CI) keepPYTHONDONTWRITEBYTECODE=true, so CI behaviour does not change.breeze down --cleanup-pycacheremoves the volume.Benchmark
Measured inside the breeze container on macOS (Docker Desktop 28.4, 16 cores,
--backend postgres --dev-mode, CI image python3.10). Time is until the api-server answers its first HTTP request, with scheduler, triggerer and dag-processor starting at the same time.airflow db migrateairflow api-server -dfirst responseairflow versionThe first run is slightly slower because the prefix also replaces the pre-built
.pycof site-packages, they are compiled once into the volume.One known limit, same as any normal Python setup:
.pycfreshness is checked by source mtime and size, so an edit within the same second that keeps the file size would reuse the old bytecode. That is why the cleanup flag exists.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Fable 5.1) following the guidelines