Fix memory leak in DagFileProcessorManager #50730
Conversation
Fix memory leak in DagFileProcessorManager Wrap DagFileProcessorProcess.start() in try/except and ensure the logger filehandle is closed on startup failure to avoid file descriptor leaks. Update DagFileStat to record import errors and prevent tight retry loops, and switch metric tags to use string file paths. Closes: apache#50708
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
|
@MaslikovEgor out of curiosity, how did you discover this was the root cause of #50708? |
I have faced the same issue with manager.py before that is why I think I am pretty familiar with this module. The leak is not big that is why I checked how resources cleaned |
Co-authored-by: Zach Liu <zachliu@users.noreply.github.com>
|
unfortunately this doesn't fix the small memory leak. i understand you are trying to properly cleanup everything upon the failure of to make sure everyone is on the same page. in #50708, i meant whether |
| return processor | ||
| except Exception as e: | ||
| # Clean up resources on failure to start | ||
| logger_filehandle.close() |
There was a problem hiding this comment.
Would it be better to put this in the finally block instead?
| processor = self._processors.pop(proc, None) | ||
| if processor: | ||
| processor.logger_filehandle.close() |
There was a problem hiding this comment.
| processor = self._processors.pop(proc, None) | |
| if processor: | |
| processor.logger_filehandle.close() | |
| if processor := self._processors.pop(proc, None): | |
| processor.logger_filehandle.close() |
There are elsewhere that can benefit from this pattern too.
|
i may have identified the leak... from importlib import metadata
from sqlalchemy import create_engineeach dag-processor's child process imports them but couldn't wipe clean the memory footprint. hence the "pinhole" leak |
Fix memory leak in DagFileProcessorManager
Wrap DagFileProcessorProcess.start() in try/except and ensure the logger filehandle is closed on startup failure to avoid file descriptor leaks. Update DagFileStat to record import errors and prevent tight retry loops, and switch metric tags to use string file paths.
Closes: #50708