Skip to content

Resolve OOM When Reading Large Logs in Webserver #49470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 67 commits into
base: main
Choose a base branch
from

Conversation

jason810496
Copy link
Member

related issue: #45079
related PR: #45129
related discussion on slack: https://apache-airflow.slack.com/archives/CCZRF2U5A/p1736767159693839

Why

In short, this PR aims to eliminate OOM issues by:

  • Replacing full log sorting with a K-Way Merge
  • Making the entire log reading path streamable (using yield generators instead of returning a list of strings)

More detailed reasoning is already described in the linked issue.

Due to too many conflicts with the old PR (#45129), this PR reworks the changes on top of the latest FileTaskHandler.

What

This PR ports the original changes from #45129 to the current version of FileTaskHandler with the following updates:

  • Fixed line-splitting error when reading in chunks using buffered line-splitting with a carry-over
  • Adopted the new log metadata structure
  • Introduced buffering for the log reader

Note: Recent Changes in FileTaskHandler

@jason810496 jason810496 self-assigned this Apr 20, 2025
@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:logging area:UI Related to UI/UX. For Frontend Developers. labels Apr 20, 2025
@jason810496 jason810496 removed the area:UI Related to UI/UX. For Frontend Developers. label Apr 20, 2025
@jason810496 jason810496 force-pushed the refactor/rework-webserver-oom-for-large-log-read branch 10 times, most recently from d47a49b to 2f5a202 Compare April 25, 2025 13:00
@jason810496 jason810496 force-pushed the refactor/rework-webserver-oom-for-large-log-read branch 2 times, most recently from 4e1bcb4 to 358f231 Compare April 26, 2025 13:22
@jason810496 jason810496 marked this pull request as ready for review April 27, 2025 05:44
@jason810496 jason810496 force-pushed the refactor/rework-webserver-oom-for-large-log-read branch from 358f231 to 9900b2e Compare April 27, 2025 05:44
@jason810496 jason810496 force-pushed the refactor/rework-webserver-oom-for-large-log-read branch from 3ee834b to c97bc9a Compare June 9, 2025 13:00
@jason810496
Copy link
Member Author

Thanks @Lee-W for the review, I will mark the PR as Draft while fixing.

@jason810496 jason810496 marked this pull request as draft June 17, 2025 07:03
jason810496 and others added 4 commits June 18, 2025 10:22
 - _is_logs_stream_like
    - add type annotation
    - reduce to 1 isinstance call
- construct log_streams in _get_compatible_log_stream inline
- use TypeDict for LogMetadata
- remove len(logs) to check empty
- revert typo of self.log_handler.read in log_reader
- log_stream_accumulator
    - refactor flush logic
    - make totoal_lines as property
    - make stream as property
@jason810496 jason810496 force-pushed the refactor/rework-webserver-oom-for-large-log-read branch from e78bdeb to 630270c Compare June 19, 2025 08:13
@jason810496 jason810496 force-pushed the refactor/rework-webserver-oom-for-large-log-read branch from 630270c to 97ab0de Compare June 20, 2025 07:27
@jason810496 jason810496 marked this pull request as ready for review June 20, 2025 07:27
@jason810496 jason810496 marked this pull request as draft June 22, 2025 11:09
- refactor structured_logs fixtures in TestLogStreamAccumulator
- use f-strign in test_file_task_handler
- assert actual value of _create_sort_key
- add details comments in test__add_log_from_parsed_log_streams_to_heap
@jason810496 jason810496 marked this pull request as ready for review June 23, 2025 01:30
@jason810496
Copy link
Member Author

Hi @eladkal, @ashb,

Would you mind taking another look at this PR when you have time?
I’d really appreciate your time, thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:logging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants