You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/09/03 13:19:32 UTC
[GitHub] [airflow] potiuk opened a new pull request #18012: Fixes memory leak in scheduler
potiuk opened a new pull request #18012:
URL: https://github.com/apache/airflow/pull/18012
The FileProcessorHandler is instantiated once and assigned a
FileHandler to write logs to files named based on context.
Apparently there are cases where single FileHandler can switch
contexts within the same process and that will lead to leaving
FileHandlers without close() method ever called on them.
This leaves the files open and memory used for those files
apparently until the files are deleted.
Fixes: #14924
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed pull request #18012: Fixes memory leak in scheduler
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #18012:
URL: https://github.com/apache/airflow/pull/18012
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #18012: Fixes memory leak in scheduler
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18012:
URL: https://github.com/apache/airflow/pull/18012#issuecomment-912564856
> Change looks good -- lets see if it fixes the problem
Yeah. Also wait to see it :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #18012: Fixes memory leak in scheduler
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18012:
URL: https://github.com/apache/airflow/pull/18012#discussion_r702387630
##########
File path: airflow/utils/log/file_processor_handler.py
##########
@@ -54,6 +54,8 @@ def set_context(self, filename):
:param filename: filename in which the dag is located
"""
local_loc = self._init_file(filename)
+ if self.handler:
+ self.handler.close()
Review comment:
Nope. That would be wrong. The whole idea is that the main handler is not closed, we should only close the underlying handler without actually closing the main one.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mrbaguvix commented on a change in pull request #18012: Fixes memory leak in scheduler
Posted by GitBox <gi...@apache.org>.
mrbaguvix commented on a change in pull request #18012:
URL: https://github.com/apache/airflow/pull/18012#discussion_r702332032
##########
File path: airflow/utils/log/file_processor_handler.py
##########
@@ -54,6 +54,8 @@ def set_context(self, filename):
:param filename: filename in which the dag is located
"""
local_loc = self._init_file(filename)
+ if self.handler:
+ self.handler.close()
Review comment:
I think we can you the method on Line 75
```suggestion
self.close()
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #18012: Fixes memory leak in scheduler
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18012:
URL: https://github.com/apache/airflow/pull/18012#issuecomment-914168976
OK. It turned out that this WAS a cache eventually. Closing this one, and I will see if we can add kernel hint to not cache the log files when we save them. I think that might help in receiving less reports of "Scheduler leaks memory" type. Users are not aware often about the ever-growing cache of linux in case of continuously saving and not removing files, so adding a hint might save our users from observing the growth in the first place and us from investigating and diagnosing the issues.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org