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

MM-27887: set traceback to crash from code #15274

Merged
merged 1 commit into from Aug 17, 2020
Merged

MM-27887: set traceback to crash from code #15274

merged 1 commit into from Aug 17, 2020

Conversation

agnivade
Copy link
Member

Unless we set traceback level to crash, goroutines from all threads aren't printed.
See: golang/go#13161.

This is important because SIGQUIT traces will miss on goroutines.

https://mattermost.atlassian.net/browse/MM-27887

Unless we set traceback level to crash, goroutines from all threads aren't printed.
See: golang/go#13161.

This is important because SIGQUIT traces will miss on goroutines.

https://mattermost.atlassian.net/browse/MM-27887
@agnivade agnivade added 2: Dev Review Requires review by a developer 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Aug 17, 2020
@wiggin77
Copy link
Member

wiggin77 commented Aug 17, 2020

golang/go#13161 mentions that after outputting the stacks it aborts when GOTRACEBACK=crash. I couldn't find any mention of that being changed. Do you know if this is still the behaviour?

Ok, I see it raises SIGABRT. That will trigger core dump and exit. I guess that's the best we can hope for.

Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@lindalumitchell lindalumitchell left a comment

Choose a reason for hiding this comment

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

Thanks, this is fine to merge without manual QA.

@lindalumitchell lindalumitchell added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review 2: Dev Review Requires review by a developer labels Aug 17, 2020
@agnivade agnivade added this to the v5.28.0 milestone Aug 17, 2020
@agnivade agnivade merged commit d48eec6 into master Aug 17, 2020
@agnivade agnivade deleted the deadlock branch August 17, 2020 16:38
@agnivade agnivade added the Docs/Needed Requires documentation label Aug 19, 2020
@agnivade
Copy link
Member Author

This change will cause the service to crash with a coredump instead of just dumping the stack trace to console.

@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Sep 17, 2020
KrishnaSindhur pushed a commit to KrishnaSindhur/mattermost-server that referenced this pull request Oct 10, 2020
Unless we set traceback level to crash, goroutines from all threads aren't printed.
See: golang/go#13161.

This is important because SIGQUIT traces will miss on goroutines.

https://mattermost.atlassian.net/browse/MM-27887
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Docs/Done Required documentation has been written
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants