-
Notifications
You must be signed in to change notification settings - Fork 94
fix: wait for graceful shutdown before restart #313
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
Conversation
to avoid failed to initialize database: resource temporarily unavailable
@mmsqe can you please link an existing github issue for this? If one does not exist, feel free to create one. We want to ensure that all PRs have an associated issue for context. |
@aljo242 to review sometime |
} | ||
return nil | ||
|
||
case err := <-errCh: | ||
if err == http.ErrServerClosed { |
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.
shouldn't we still return nil from here and not log "closed" as an error?
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.
excluded
var app types.Application | ||
defer func() { | ||
if err := db.Close(); err != nil { | ||
svrCtx.Logger.Error("error closing db", "error", err.Error()) | ||
if app == nil { |
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.
won't this always be true?
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.
handle db close if app not initiated when openTraceWriter fail
Line 240 in a995466
if err != nil { |
var app types.Application | ||
defer func() { | ||
if err := db.Close(); err != nil { | ||
svrCtx.Logger.With("error", err).Error("error closing db") | ||
if app == nil { |
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.
ditto
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.
@mmsqe left comments
Description
to avoid failed to initialize database: resource temporarily unavailable when node restart without waiting for gracefully shutdown
Closes: #319
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
main
branchReviewers Checklist
All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.
I have...
Unreleased
section inCHANGELOG.md