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

Exit main process if error group goroutine returns error #671

Merged
merged 7 commits into from
Jan 24, 2024

Conversation

haoming29
Copy link
Contributor

@haoming29 haoming29 commented Jan 17, 2024

Fixes #661

  • Fix the issue where Pelican won'y exit main process if error group goroutine returns error, unless an error returns in the main goroutine
  • Fix the panic at the director when a cache advertise itself to the director, as we didn't have the case for cache when doing version compatibility check
  • Improve logging at program exit to be consistent.
  • Improve cache error group, remove duplicated exit handler that root.go already had.

However, this didn't fix the issue where when we exit the program with ctrl+c , xrootd reported error as it treated SIGINT similar as SIGSEGV. We might have to refactor some of the code in daemon/launch_unix.go. Created a separate issue for this: #672

@haoming29 haoming29 added the bug Something isn't working label Jan 17, 2024
@haoming29 haoming29 added this to the v7.5.0 milestone Jan 17, 2024
@haoming29 haoming29 added the enhancement New feature or request label Jan 17, 2024
@haoming29 haoming29 requested a review from bbockelm January 17, 2024 22:29
Copy link
Contributor

@matyasselmeci matyasselmeci left a comment

Choose a reason for hiding this comment

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

Some tweaks; also the changes in director/redirect.go seem unrelated to this issue and should be in a separate PR.

Copy link
Collaborator

@bbockelm bbockelm left a comment

Choose a reason for hiding this comment

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

A few changes requested. Also spotted another way shutdown could get stuck that's specific to the cache; should fix it while we're working in this area.

Copy link
Collaborator

@bbockelm bbockelm left a comment

Choose a reason for hiding this comment

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

LGTM with latest revisions.

@joereuss12 - I believe you were the 7.4.0 release manager, right? If so, can you please backport this PR to that branch and do a bugfix release for @matyasselmeci?

@haoming29 haoming29 merged commit 91974fd into PelicanPlatform:main Jan 24, 2024
9 checks passed
@haoming29 haoming29 deleted the exit-with-error branch January 24, 2024 15:35
@joereuss12
Copy link
Contributor

Just cut a bugfix release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache: deal with xrootd processes dying
5 participants