Skip to content

Conversation

@guan404ming
Copy link
Member

@guan404ming guan404ming commented Aug 13, 2025

Related PR

Why

  • using application/x-ndjson would reduce the load of api-server when processing large log

How

  • make application/x-ndjson as default but it would not change any previous user experience

before
image

after
image


^ 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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@bbovenzi
Copy link
Contributor

Why would a user want to turn streaming off?

@guan404ming
Copy link
Member Author

guan404ming commented Aug 14, 2025

Why would a user want to turn streaming off?

I’ve also wondered about the possibility of this. I’m not sure, so I added the functionality first. But I feel like there’s probably no real need for it, so I've removed it.

@guan404ming guan404ming force-pushed the streaming-log branch 2 times, most recently from cec82fc to 5739493 Compare August 14, 2025 14:11
@bbovenzi
Copy link
Contributor

Is this actually streaming and rendering logs as they stream in or just reading the first stream? I suspect it is the latter but I am a bit new to this format.

@jason810496
Copy link
Member

Is this actually streaming and rendering logs as they stream

The API should stream the log until the end, but there are still some errors with this process. I will work on a fix later. The root cause is that when the FileTaskHandler opens the file, it can only read content that has already been flushed to the file, and cannot access content that is still being written.

There should be a polling mechanism to check for new changes to the file, or perhaps the connection could remain open so the API can continuously poll for new content and stream it to the frontend.

@jason810496
Copy link
Member

The API should stream the log until the end, but there are still some errors with this process. I will work on a fix later. The root cause is that when the FileTaskHandler opens the file, it can only read content that has already been flushed to the file, and cannot access content that is still being written.

Regardless of this error, the frontend should adapt to application/nd-json, which is the main change in this PR. The previous application/json format is very resource-intensive for the API server and may cause OOM issues when logs are large, even after the fix in #49470. This is because JSON serialization requires the entire log content to be stored in a single large dictionary.

@guan404ming
Copy link
Member Author

guan404ming commented Aug 15, 2025

Is this actually streaming and rendering logs as they stream in or just reading the first stream? I suspect it is the latter but I am a bit new to this format.

Yes, this is not real streaming. I misunderstood and also confirmed more details with @jason810496. After offline discussion and test, we confirmed that the streaming implementation still has some issues that need to be addressed. But switching from application/json to application/x-ndjson would still help reduce the load on the api-server like Jason said. So I’ll update this PR for only changing the header for now and will implement actual streaming once the issues are resolved.

@guan404ming guan404ming changed the title Enable streaming log in UI Update useLog to support application/x-ndjson Aug 15, 2025
@bbovenzi
Copy link
Contributor

bbovenzi commented Aug 18, 2025

Thanks for the clarification. lgtm with the updated title

@bbovenzi bbovenzi merged commit 2deeb8c into apache:main Aug 18, 2025
53 checks passed
Sudi-Lyu pushed a commit to Sudi-Lyu/airflow that referenced this pull request Aug 21, 2025
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
@eladkal eladkal added this to the Airflow 3.1.0 milestone Sep 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:translations area:UI Related to UI/UX. For Frontend Developers. translation:default

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants