zozo123 commented on PR #74173: URL: https://github.com/apache/airflow/pull/74173#issuecomment-6254253810
Thanks for the thorough review. Everything inline is addressed in 7c367ee0e2, 2d3dec6b5c and 08c9943712. Below are the measurements you asked for. They are taken from the composite-step durations (`end-action … duration_ms`) in the job logs of 79 CI-image consumer jobs per run. The PR runs are 37950653129, 37980143706 and 39072732374 (attempt 2). For comparison, the baseline is three full-CI PR runs without the snapshot, from the same day as the latest PR run: 39141869252, 39143583838 and 39150120169. | per consumer job, median | download | load / unpack | total | |---|---|---|---| | stash path (3 baseline runs) | 41–64 s | `breeze ci-image load` 103–104 s | 154–175 s | | snapshot path (3 PR runs) | 71–155 s | unpack 30–34 s | 109–195 s | - **Acceptance:** in each of the three runs, all 79 eligible jobs restored the snapshot, with 0 fallbacks. The other 26 jobs with a prepare step use the PROD image and are not eligible. - **Builder:** prune and create took 85–105 s and the upload 12–17 s. Both happen in the build job, so they sit on the critical path of every consumer. - **Storage:** 2.55 GB per Python × platform (zstd -3), against 2.38 GB for the stash. Retention is 1 day. **What the numbers show:** - Unpacking is consistently about 70 s faster than `docker load`. - But the snapshot download was slower than the stash download in all three runs, at 16–36 MB/s, which ate most of that gain. Per job, the result ranged from 59 s faster to 27 s slower than the ~168 s baseline. - The builder also adds 1.7–2 min of wall clock before any consumer starts. - So on these numbers the net effect is not clearly positive. The runs are also from different days, so download throughput is not a like-for-like comparison. The runs on 08c9943712 also include the new costs: the metadata download, the check, and the file-tree hash after the restore. I'll post those numbers when they finish. If they don't show a clear net win, the extra artifact storage isn't worth it and I'm happy to close this. **@shahar1:** - Snapshots now run only when `runner.environment == 'github-hosted'`, on both the producer and the consumer side, so #74889 can land in either order. - CodeQL alert 670 is the same `poisonable-step` pattern as the alerts already dismissed on `main`, and still needs a maintainer to dismiss it. -- 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]
