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

Mitigate panic in profiler shutdown, bound timeout #1932

Merged
merged 1 commit into from
Feb 17, 2023
Merged

Conversation

morrisonlevi
Copy link
Collaborator

@morrisonlevi morrisonlevi commented Feb 16, 2023

Description

This mitigation is for issue #1919. It doesn't fix the root cause, which we haven't been able to determine yet.

This also bounds our shutdown timeouts. Before, they ought to have been bounded as a consequence of how the code was written, but this is explicit. It's also a tighter bound than before, 2 seconds instead of 10 seconds.

Also, the new Rust 1.64 formatter apparently formats some things differently, so those are included as well.

Readiness checklist

  • Changelog has been added to the release document.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.

This mitigation is for issue #1919. It doesn't fix the root cause,
which we haven't been able to determine yet.

This also bounds our shutdown timeouts. Before, they _ought_ to have
been bounded as a consequence of how the code was written, but this
is explicit. It's also a tighter bound than before, 2 seconds instead
of 10 seconds.
@morrisonlevi morrisonlevi added the profiling Relates to the Continuous Profiler label Feb 16, 2023
@morrisonlevi morrisonlevi added this to the 0.85.0 milestone Feb 16, 2023
@morrisonlevi morrisonlevi requested a review from a team as a code owner February 16, 2023 17:20
Copy link
Member

@realFlowControl realFlowControl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@realFlowControl realFlowControl merged commit 3ae45ea into master Feb 17, 2023
@realFlowControl realFlowControl deleted the levi/join branch February 17, 2023 09:21
@bwoebi bwoebi mentioned this pull request Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
profiling Relates to the Continuous Profiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants