seanmuth commented on code in PR #69832:
URL: https://github.com/apache/airflow/pull/69832#discussion_r3615454145
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/grid.py:
##########
@@ -212,24 +225,33 @@ def get_dag_structure(
.distinct()
),
)
- .execution_options(yield_per=5) # balance between peak memory usage
and round trips
Review Comment:
For visibility since this may need more eyes: this is functionally resolved
already. Ephraim raised the same concern (that materializing all historical
serdags regresses the `yield_per=5` memory-bounded behavior from the prior
investigation), and c58da2916e reworked the fetch to preserve that property.
Current approach: fetch just the serdag ids (cheap), then process them in
batches of 5 (matching the original `yield_per=5` batch size), each batch
loaded and detached inside its own short-lived `create_session()` that's closed
*before* that batch is deserialized. So peak memory stays bounded to one batch,
same as before, and the DB connection is additionally released during the
CPU-bound deserialize/merge instead of being held open across it (the change
this PR is about).
So the memory-growth concern this thread flagged is addressed by the current
diff, not overridden by it. Happy to dig further if there's a specific scenario
from the original investigation this doesn't cover.
--
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]
