aeroyorch commented on PR #73044: URL: https://github.com/apache/airflow/pull/73044#issuecomment-6010165516
> Approving the change itself. Streaming with a `pending` list of byte pieces avoids both the quadratic concatenation and `readline()`'s `LineTooLong`, decoding after joining keeps multi-byte characters intact, and `aclosing` releases the response on the early break. Catching `ClientError` in the trigger also covers aiohttp's read timeouts (`SocketTimeoutError` subclasses it), so a failed read no longer re-defers and re-reads the whole log. I rebased the branch onto `main`. > > **Please don't merge yet, though.** The inline comment on `pod_manager.py` about moving log emission back onto the event loop is blocking until someone who knows the Airflow 3 trigger logging path confirms it's fine. @ashb @amoghrajesh, could one of you take a look at that thread? @aeroyorch, since you moved this step into a thread in #69661, your view on what changed since then would help too. > > --- > Drafted-by: Claude Code (Opus 5.5); reviewed by @potiuk before posting Well, what I noticed was that log emission improved after moving to a thread, with fewer warnings about the main triggerer loop being busy. But due to an out of control pod emitting tons of logs, suddenly the triggerer's memory usage started to grow unbounded, first due to the issue with the last log timestamp being kept only in memory, and secondly due to read_logs keeping the raw bytes, decoded string and lines in memory at the same time, and then queuing them on the default executor, whose queue is unbounded. I'd have liked to run an analysis with memray, but I couldn't this time. In any case, I think this is the correct way of making Airflow robust against heavy workloads without affecting, or reducing the impact on, others. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
