[v2-11-test] Resolve OOM When Reading Large Logs in Webserver #45914
Conversation
boring-cyborg
bot
added
area:API
jason810496
force-pushed
the
refactor/webserver-oom-for-large-log-read-back-port
branch
5 times, most recently
from
cc217a5 to
e9e1891
Compare
jason810496
requested review from
XD-DENG,
ashb,
bbovenzi,
eladkal,
ephraimbuddy,
kaxil,
o-nikolas,
pierrejeambrun and
ryanahamilton
as code owners
jason810496
force-pushed
the
refactor/webserver-oom-for-large-log-read-back-port
branch
from
f08efca to
2c1a3cc
Compare
jason810496
requested review from
Fokko,
bolkedebruin,
dstandish,
hussein-awala,
jedcunningham,
josh-fell,
mik-laj,
mobuchowski and
potiuk
as code owners
potiuk
left a comment
potiuk
left a comment
There was a problem hiding this comment.
Looks great - few nits, but I think this is a great improvement.
potiuk
added
the
backport-to-v3-1-test
|
I marked it as 3.0.1 - because technically - it is bugfix, though we might refrain from merging it to 3.0.1 - up to @kaxil to decide, but I would be inclined to include it, especially if we decide to also release it in 2.11 - it would be strange to have it in 2.11.0 but not have it in 3.0.* |
|
Hi @potiuk, #49470 targets the 3.0+ branch, since there are significant changes to Backporting this one (#45914) directly would likely result in conflicts. However, it should be no problem to backport #49470 to the |
jason810496
force-pushed
the
refactor/webserver-oom-for-large-log-read-back-port
branch
2 times, most recently
from
133624d to
09799f4
Compare
kaxil
changed the title
Bumps [trove-classifiers](https://github.com/pypa/trove-classifiers) from 2025.4.11.15 to 2025.4.28.22. - [Release notes](https://github.com/pypa/trove-classifiers/releases) - [Commits](pypa/trove-classifiers@2025.4.11.15...2025.4.28.22) --- updated-dependencies: - dependency-name: trove-classifiers dependency-version: 2025.4.28.22 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- test_interleave_interleaves - test_interleave_logs_correct_ordering - test__read_for_celery_executor_fallbacks_to_worker
kaxil
force-pushed
the
refactor/webserver-oom-for-large-log-read-back-port
branch
from
09799f4 to
246e4fb
Compare
| message = str(next(iter(log_streams[0]))) | ||
| while metadata["end_of_log"] is False: | ||
| logs, metadata = task_log_reader.read_log_chunks( | ||
| hosts, log_streams, metadata = task_log_reader.read_log_chunks( |
There was a problem hiding this comment.
Since this is a big change, I am afraid to include it in 2.11!
For example, in this case where the return type of task_log_reader.read_log_chunks has changed.
There was a problem hiding this comment.
@kaxil it seems he is intending this to go to 3.0 only
#45914 (comment)
There was a problem hiding this comment.
yeah we should not spend time adding meaningful changes to 2.x
There was a problem hiding this comment.
For example, in this case where the return type of task_log_reader.read_log_chunks has changed.
I think changing the return type of read_log_chunks or other internal methods isn't a problem, since the user-facing interface is the REST API. As long as the REST API response remains consistent, users won’t be affected or need to be aware of the refactor.
|
Great PR @jason810496 but I am worried about backwards compatibility with this change -- especially for cases where an external provider has built upon the 2.10.5 logging format or clients using cc @dstandish @uranusjr @ashb Any opinion from you folks since you are more familiar with our logging architecture? My recommendation is to not include this in 2.x and close this and review #49470 for 3.1 |
jason810496
force-pushed
the
refactor/webserver-oom-for-large-log-read-back-port
branch
from
246e4fb to
6cef899
Compare
Just clarify this change about break backward compatibility for log serving.
That works for me as well. |
|
Thanks @jason810496 , will review #49470 |
6 participants
related issue: #45079
related PR: #45129
related discussion on slack: https://apache-airflow.slack.com/archives/CCZRF2U5A/p1736767159693839
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.