Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add "enable_tracemalloc" to log memory usage in scheduler #42304

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

Lee-W
Copy link
Member

@Lee-W Lee-W commented Sep 18, 2024

Why

add more information to ease the debugging process when receiving sigurs1

What

log memory usage in the scheduler when handling sigurs1 if enable_tracemalloc is set


^ 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 newsfragments.

@boring-cyborg boring-cyborg bot added the area:Executors-core LocalExecutor & SequentialExecutor label Sep 18, 2024
@Lee-W Lee-W force-pushed the log-memory-usage-in-executor branch from a8cf17a to 018f672 Compare September 18, 2024 13:55
@Lee-W Lee-W changed the title feat(executor): log memory useage in executor feat(executor): log memory useage in executor when receive sigurs2 Sep 18, 2024
@Lee-W Lee-W changed the title feat(executor): log memory useage in executor when receive sigurs2 feat(executor): log memory usage in executor Sep 18, 2024
@Lee-W Lee-W force-pushed the log-memory-usage-in-executor branch from 018f672 to 32662af Compare September 19, 2024 02:18
airflow/executors/base_executor.py Outdated Show resolved Hide resolved
airflow/executors/base_executor.py Outdated Show resolved Hide resolved
@Lee-W Lee-W force-pushed the log-memory-usage-in-executor branch 2 times, most recently from 41f7848 to 91bc47f Compare September 19, 2024 13:39
@jedcunningham
Copy link
Member

Random thought: Maybe we move this tracemalloc behavior into sigusr1? Just seems like maybe having a debug dump like we do today in sigusr2 and keeping performance impacts to stuff in sigusr1 might be a better approach.

@Lee-W Lee-W force-pushed the log-memory-usage-in-executor branch from 0b43901 to 867908b Compare September 20, 2024 07:49
@Lee-W
Copy link
Member Author

Lee-W commented Sep 20, 2024

Random thought: Maybe we move this tracemalloc behavior into sigusr1? Just seems like maybe having a debug dump like we do today in sigusr2 and keeping performance impacts to stuff in sigusr1 might be a better approach.

Sounds good! Just move it to SIGUSR1. Another thing I just noticed is that this might not be helpful if we start when receiving a signal. We probably still need to start it before that. Do you think making it a config makes sense?

@Lee-W Lee-W force-pushed the log-memory-usage-in-executor branch 2 times, most recently from 66268c7 to c74f569 Compare September 23, 2024 08:57
@Lee-W Lee-W force-pushed the log-memory-usage-in-executor branch from c74f569 to 431ea75 Compare September 23, 2024 13:38
@Lee-W Lee-W changed the title feat(executor): log memory usage in executor add "enable_tracemalloc" to log memory usage in scheduler Sep 24, 2024
@Lee-W
Copy link
Member Author

Lee-W commented Sep 24, 2024

I'm thinking of merging this one later today if no one has more thoughts. Thanks!

@Lee-W Lee-W merged commit 57eed58 into apache:main Sep 24, 2024
51 checks passed
@Lee-W Lee-W deleted the log-memory-usage-in-executor branch September 24, 2024 11:20
joaopamaral pushed a commit to joaopamaral/airflow that referenced this pull request Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Executors-core LocalExecutor & SequentialExecutor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants