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

core,eth,les: update unclean shutdown markers regularly #24077

Merged
merged 7 commits into from Dec 17, 2021

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Dec 7, 2021

Successor to #22974, fixes #22580

eth/backend.go Outdated
func (s *Ethereum) updateUncleanShutdownMarkers() {
// update marker every five minutes
ticker := time.NewTicker(300 * time.Second)
defer func() { ticker.Stop() }()
Copy link
Member

Choose a reason for hiding this comment

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

you can simplify the code by defer ticker.Stop()

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@s1na s1na requested a review from fjl as a code owner December 14, 2021 16:20
@s1na
Copy link
Contributor Author

s1na commented Dec 14, 2021

Updated based on the feedback I got on triage.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM!

@holiman holiman added this to the 1.10.14 milestone Dec 15, 2021
@fjl
Copy link
Contributor

fjl commented Dec 15, 2021

Package name 'node' is not great because it conflicts with the existing top-level package name we already have. Maybe call it 'shutdowncheck' or something. It doesn't need to be a good/short name.

@s1na
Copy link
Contributor Author

s1na commented Dec 16, 2021

I wanted to make it more general so we could put other modules in there too. But yeah the name conflict wasn't great. I renamed it.

@s1na
Copy link
Contributor Author

s1na commented Dec 17, 2021

Updated based on standup discussion. I could've avoided MarkStartup by having that logic in New, but somehow that didn't feel clean.

@fjl fjl removed the status:triage label Dec 17, 2021
@fjl fjl merged commit ada9c77 into ethereum:master Dec 17, 2021
sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Dec 19, 2021
@s1na s1na deleted the unclean-shutdown branch January 5, 2022 16:55
JacekGlen pushed a commit to JacekGlen/go-ethereum that referenced this pull request May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the unclean shutdown marker every 5 minutes
5 participants