Skip log accumulator disk spill when reading finished task logs by jason810496 · Pull Request #70671 · apache/airflow

jason810496 · 2026-07-29T10:17:01Z

part of the streaming task log series

related: Add streaming task log support to BaseExecutor and FileTaskHandler #69299 (merged core plumbing), Add streaming task log support to KubernetesExecutor #69300 (merged KubernetesExecutor counterpart)

Why

FileTaskHandler._read wraps every read in LogStreamAccumulator, which eagerly drains the whole merged log stream (5000 lines to memory, the rest to a tempfile on the API server) before the first byte reaches the client — only to get the total_lines for the continuation token.

For a finished task no continuation token is ever issued (end_of_log=True on both the JSON and NDJSON paths), so the count is dead weight: every terminal-task read full-log downloads — pays a full-log disk write + read-back + re-parse and a time-to-first-byte equal to the whole drain, for nothing.

What

Early-return in FileTaskHandler._read bypassing LogStreamAccumulator when end_of_log is True.
Memory stays bounded either way — the k-way merge heap in _interleave_logs (capped at HEAP_DUMP_SIZE, flushing half when full) is what bounds the read path, not the accumulator.

Was generative AI tooling used to co-author this PR?

Yes, with help of Claude Code Sonnet 5 following the guidelines

Finished-task reads drop the log_pos continuation token from 3.4.0 onward, but the provider test matrix still runs against released cores (3.0-3.3) that emit it. Gate the S3, GCS, and WASB task-handler metadata assertions on AIRFLOW_V_3_4_0_PLUS so both the source run and the older-core compatibility runs pass.

Apply the log_pos skip and header once for both paths instead of duplicating it in the finished-read branch. Co-Authored-By: Claude <noreply@anthropic.com>

Co-Authored-By: Claude <noreply@anthropic.com>

jason810496 · 2026-10-20T08:23:39Z

Encountered unrelated CI failure, this PR is ready to review.

ashb

i'm not sure the tests cover the distinction between the two paths well enough. Can we check does/doesnt accumulation happen

pierrejeambrun

+1 for testing both paths more explicitely.

Otherwise looks good to me.

Cover running, deferred, older-try, and finished reads, including a running task's follow-up poll with an inbound log_pos. Update the _read docstring for the log_pos contract. Co-Authored-By: Claude <noreply@anthropic.com>

jason810496

Thanks for the review, I added test__read_accumulates_only_unfinished_reads in e4d36e3.

Co-Authored-By: Claude <noreply@anthropic.com>

log_pos is a core detail covered by the core tests, so the provider tests no longer need a version gate for it. Co-Authored-By: Claude <noreply@anthropic.com>

jason810496

Thanks for the review.

uranusjr

FileTaskHandler.read() docstring still says “streaming”; we should fix it.

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>

github-actions · 2026-10-29T12:27:07Z

Backport successfully created: v3-4-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status	Branch	Result
✅	v3-4-test

@uranusjr

…k logs (#70671) * Skip log accumulator disk spill when reading finished task logs * Version-gate remote log handler tests for terminal-state metadata Finished-task reads drop the log_pos continuation token from 3.4.0 onward, but the provider test matrix still runs against released cores (3.0-3.3) that emit it. Gate the S3, GCS, and WASB task-handler metadata assertions on AIRFLOW_V_3_4_0_PLUS so both the source run and the older-core compatibility runs pass. * Add plan for incremental UI log fetch via continuation token * Wrap only unfinished log reads in the stream accumulator Apply the log_pos skip and header once for both paths instead of duplicating it in the finished-read branch. * Drop the accidentally committed planning doc * Test both accumulator paths of FileTaskHandler._read Cover running, deferred, older-try, and finished reads, including a running task's follow-up poll with an inbound log_pos. Update the _read docstring for the log_pos contract. * Drop the misleading log_pos note in the log read test * Drop the redundant accumulator comment in the log read test * Assert only end_of_log in the remote log handler tests log_pos is a core detail covered by the core tests, so the provider tests no longer need a version gate for it. * Apply suggestion from @uranusjr to fix the comment Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com> --------- (cherry picked from commit 97d4d4e) Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com> Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>

@uranusjr

…k logs (#70671) (#75956) * Skip log accumulator disk spill when reading finished task logs * Version-gate remote log handler tests for terminal-state metadata Finished-task reads drop the log_pos continuation token from 3.4.0 onward, but the provider test matrix still runs against released cores (3.0-3.3) that emit it. Gate the S3, GCS, and WASB task-handler metadata assertions on AIRFLOW_V_3_4_0_PLUS so both the source run and the older-core compatibility runs pass. * Add plan for incremental UI log fetch via continuation token * Wrap only unfinished log reads in the stream accumulator Apply the log_pos skip and header once for both paths instead of duplicating it in the finished-read branch. * Drop the accidentally committed planning doc * Test both accumulator paths of FileTaskHandler._read Cover running, deferred, older-try, and finished reads, including a running task's follow-up poll with an inbound log_pos. Update the _read docstring for the log_pos contract. * Drop the misleading log_pos note in the log read test * Drop the redundant accumulator comment in the log read test * Assert only end_of_log in the remote log handler tests log_pos is a core detail covered by the core tests, so the provider tests no longer need a version gate for it. * Apply suggestion from @uranusjr to fix the comment --------- (cherry picked from commit 97d4d4e) Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com> Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>

boring-cyborg Bot added the area:logging label Jul 29, 2026

jason810496 self-assigned this Jul 29, 2026

jason810496 added the area:API Airflow's REST/HTTP API label Jul 29, 2026

jason810496 marked this pull request as ready for review July 29, 2026 10:17

jason810496 requested review from Lee-W, ashb, gopidesupavan and pierrejeambrun July 29, 2026 10:17

jason810496 force-pushed the refactor/logging/bypass-accumulator-terminal-reads branch from ff86435 to d9fbcfb Compare July 30, 2026 03:21

jason810496 requested a review from o-nikolas as a code owner July 31, 2026 11:33

jason810496 force-pushed the refactor/logging/bypass-accumulator-terminal-reads branch from 0fa14b3 to b0d2376 Compare July 31, 2026 15:12

jason810496 requested a review from shahar1 as a code owner July 31, 2026 15:12

jason810496 added this to the Airflow 3.4.0 milestone Aug 5, 2026

jason810496 force-pushed the refactor/logging/bypass-accumulator-terminal-reads branch from 99715a3 to 686cbfa Compare September 7, 2026 02:37

guan404ming reviewed Sep 30, 2026

View reviewed changes

Comment thread ui-continue-token-plan.md Resolved

Comment thread airflow-core/src/airflow/utils/log/file_task_handler.py Resolved

jason810496 added 3 commits October 11, 2026 09:47

Skip log accumulator disk spill when reading finished task logs

1f070a4

Add plan for incremental UI log fetch via continuation token

801e98f

jason810496 force-pushed the refactor/logging/bypass-accumulator-terminal-reads branch from 686cbfa to 801e98f Compare October 11, 2026 09:49

jason810496 and others added 2 commits October 12, 2026 11:20

Wrap only unfinished log reads in the stream accumulator

3532cec

Apply the log_pos skip and header once for both paths instead of duplicating it in the finished-read branch. Co-Authored-By: Claude <noreply@anthropic.com>

Drop the accidentally committed planning doc

07c855d

Co-Authored-By: Claude <noreply@anthropic.com>

vatsrahul1001 added the backport-to-v3-4-test Backport to v3-4-test label Oct 18, 2026

ashb reviewed Oct 20, 2026

View reviewed changes

pierrejeambrun reviewed Oct 20, 2026

View reviewed changes

kaxil reviewed Oct 20, 2026

View reviewed changes

Test both accumulator paths of FileTaskHandler._read

e4d36e3

Cover running, deferred, older-try, and finished reads, including a running task's follow-up poll with an inbound log_pos. Update the _read docstring for the log_pos contract. Co-Authored-By: Claude <noreply@anthropic.com>

jason810496 mentioned this pull request Oct 20, 2026

Skip log accumulator on NDJSON reads of running task logs #75211

Open

jason810496 commented Oct 20, 2026

View reviewed changes

jason810496 requested a review from pierrejeambrun October 20, 2026 12:27

jason810496 requested review from ashb, guan404ming and kaxil October 20, 2026 12:27

jason810496 modified the milestones: Airflow 3.4.0, Airflow 3.4.1 Oct 23, 2026

uranusjr reviewed Oct 28, 2026

View reviewed changes

Comment thread airflow-core/src/airflow/utils/log/file_task_handler.py Resolved

uranusjr reviewed Oct 28, 2026

View reviewed changes

Comment thread airflow-core/tests/unit/utils/test_log_handlers.py Resolved

uranusjr reviewed Oct 28, 2026

View reviewed changes

Comment thread airflow-core/tests/unit/utils/test_log_handlers.py Resolved

uranusjr reviewed Oct 28, 2026

View reviewed changes

Comment thread providers/amazon/tests/unit/amazon/aws/log/test_s3_task_handler.py Resolved

jason810496 and others added 3 commits October 29, 2026 02:27

Drop the misleading log_pos note in the log read test

4c91c71

Co-Authored-By: Claude <noreply@anthropic.com>

Drop the redundant accumulator comment in the log read test

28a2b9c

Co-Authored-By: Claude <noreply@anthropic.com>

Assert only end_of_log in the remote log handler tests

589d201

log_pos is a core detail covered by the core tests, so the provider tests no longer need a version gate for it. Co-Authored-By: Claude <noreply@anthropic.com>

jason810496 commented Oct 29, 2026

View reviewed changes

jason810496 requested a review from uranusjr October 29, 2026 05:27

uranusjr approved these changes Oct 29, 2026

View reviewed changes

uranusjr reviewed Oct 29, 2026

View reviewed changes

Comment thread airflow-core/src/airflow/utils/log/file_task_handler.py Resolved

Apply suggestion from @uranusjr to fix the comment

3bebdb7

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>

jason810496 merged commit 97d4d4e into apache:main Oct 29, 2026
102 checks passed

jason810496 deleted the refactor/logging/bypass-accumulator-terminal-reads branch October 29, 2026 12:25

mungiyo mentioned this pull request Nov 2, 2026

Stream remote task logs from S3 instead of reading whole objects #73517

Merged

Conversation

jason810496 commented Jul 29, 2026

Why

What

Was generative AI tooling used to co-author this PR?

Uh oh!

Uh oh!

Uh oh!

jason810496 commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ashb left a comment

Choose a reason for hiding this comment

Uh oh!

pierrejeambrun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jason810496 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jason810496 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

uranusjr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 29, 2026

Backport successfully created: v3-4-test

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jason810496 commented Oct 20, 2026 •

edited

Loading

jason810496 left a comment •

edited

Loading