-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adjust heartbeat behavior #180
base: v1.10.2+RAI
Are you sure you want to change the base?
Conversation
`jl_print_task_backtraces()` can take long enough that there can be heartbeat loss, which can trigger printing task backtraces again, unless it is called from the heartbeat thread which takes care of that possible problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
if (!jl_inside_heartbeat_thread()) { | ||
jl_heartbeat_pause(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 Why the if-check?
If a heartbeat loss causes us to print backtraces, could the backtraces we're printing cause another heartbeat loss?
I think I remember you said the answer is "no" when we discussed over zoom, but it would be nice to clarify that in the comments, here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I'd written that up. Sorry, I'll add a comment. If this is called from the heartbeat thread, there will be no additional heartbeat loss because it is the heartbeat thread itself that determines whether there is heartbeat loss. So when it returns from here, the counter for heartbeats missed is unchanged.
PR Description
Introduce a heartbeat pause/resume capability. Use this capability:
Checklist
Requirements for merging:
port-to-*
labels that don't apply.