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

Detect unclean shutdowns #20859

Closed
ligi opened this issue Apr 2, 2020 · 10 comments
Closed

Detect unclean shutdowns #20859

ligi opened this issue Apr 2, 2020 · 10 comments

Comments

@ligi
Copy link
Member

ligi commented Apr 2, 2020

it would be nice if geth could detect if geth was not shutdown correctly (e.g. it crashed)
then the user could be offered the upcoming tooling: #20858. Also it will help us investigate issues like #20845

@MariusVanDerWijden
Copy link
Member

This is a great first issue if someone wants to pick it up! I've created a PoC here: https://github.com/MariusVanDerWijden/go-ethereum/tree/unclean-shutdowns
Problem with this is

  • The logic is at the wrong place
  • The code is not really clean
  • If we detected an unclean shutdown at the start, the shutdown should not be erased after the next successful shutdown
  • The code does not use the nice logging style

Feel free to pick this issue up if you want to start contributing to go-ethereum
If you need any help, ping me on gitter or here

@muhammednagy
Copy link
Contributor

muhammednagy commented May 19, 2020

Hey i can pick this issue up if no one else has taken it.
Feel free to assign it to me @MariusVanDerWijden

@ligi
Copy link
Member Author

ligi commented May 19, 2020

@muhammednagy great! I just assigned you - enjoy working on it and let us know if you run into problems

@muhammednagy
Copy link
Contributor

muhammednagy commented May 20, 2020

@MariusVanDerWijden

If we detected an unclean shutdown at the start, the shutdown should not be erased after the next successful shutdown

When should it be erased then?
or should it be not erased at all?
could probably add timestamp to the unclean shutdown file name.

could change it from unclean-shutdown-timestamp to read-unclean-shutdown-timestamp if we don't want to delete it.

@ligi
Copy link
Member Author

ligi commented May 21, 2020

IMHO we should not throw away any of these files automatically. These can be very valuable to debug problems and the more info we can get the better I would say. And yes a timestamp in the file is a good idea

@karalabe
Copy link
Member

I would not put these into external files at all. Placing them into leveldb is a much much better idea because we don't have to track yet another files which need to be placed in the correct datadir, etc. We did the same with the snapshot diffs and it makes things so much cleaner all around.

@ligi
Copy link
Member Author

ligi commented May 21, 2020

Also an option - but we need to make sure a corrupted database will not be a blocker from accessing these logs.

@karalabe
Copy link
Member

The database doesn't get corrupted (unless your hard drive fails).

@muhammednagy
Copy link
Contributor

I think database is a better option.
I will start working on that.
Thanks @karalabe & @ligi

@FridayOrtiz
Copy link

For anyone else experiencing this, the fix is pending PR #21893.

@ligi ligi closed this as completed Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants