Skip to content

Fix duplicate log reads when resuming from log_pos#63531

Merged
gopidesupavan merged 1 commit intoapache:mainfrom
gopidesupavan:fix-log-position-handling
Mar 13, 2026
Merged

Fix duplicate log reads when resuming from log_pos#63531
gopidesupavan merged 1 commit intoapache:mainfrom
gopidesupavan:fix-log-position-handling

Conversation

@gopidesupavan
Copy link
Member

Fix FileTaskHandler._read() so resumed log reads actually honor metadata["log_pos"].

Previously we called islice(...) without reassigning the returned iterator, so no log lines were skipped and resumed reads could return duplicate content from the beginning of the stream.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@eladkal eladkal added this to the Airflow 3.1.9 milestone Mar 13, 2026
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Mar 13, 2026
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! It's a long time bug introduced in 3.0, thanks a lot!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes FileTaskHandler._read() so resumed log reads correctly honor metadata["log_pos"], preventing duplicate log content when tailing/resuming from a prior position.

Changes:

  • Reassigns the islice(...) result to actually advance the output stream when log_pos is provided.
  • Adds a unit test that verifies resumed reads skip previously-returned log lines and return only new content.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
airflow-core/src/airflow/utils/log/file_task_handler.py Fixes resume/tailing behavior by correctly slicing the stream based on metadata["log_pos"].
airflow-core/tests/unit/utils/test_log_handlers.py Adds regression coverage ensuring resumed reads don’t return duplicate earlier log lines.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@kaxil
Copy link
Member

kaxil commented Mar 13, 2026

CI Test failures look unrelated but worth veriyfing

@gopidesupavan
Copy link
Member Author

CI Test failures look unrelated but worth veriyfing

yeah looks like tests are failing for the lowest dep likely the pydantic==2.12.0 causing issues https://github.com/apache/airflow/actions/runs/23058551095/job/66982917534?pr=63531#step:8:1938

@gopidesupavan
Copy link
Member Author

failures test pr is here #63570

@gopidesupavan gopidesupavan merged commit 00dc420 into apache:main Mar 13, 2026
132 of 135 checks passed
@gopidesupavan gopidesupavan deleted the fix-log-position-handling branch March 13, 2026 23:02
github-actions bot pushed a commit that referenced this pull request Mar 13, 2026
(cherry picked from commit 00dc420)

Co-authored-by: GPK <gopidesupavan@gmail.com>
@github-actions
Copy link

Backport successfully created: v3-1-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-1-test PR Link

jason810496 pushed a commit that referenced this pull request Mar 14, 2026
… (#63571)

(cherry picked from commit 00dc420)

Co-authored-by: GPK <gopidesupavan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:logging type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants