From ada9c774e95f308495132557772a00d31cc1797c Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi <1591639+s1na@users.noreply.github.com> Date: Fri, 17 Dec 2021 17:48:51 +0330 Subject: [PATCH] eth, les: update unclean shutdown markers regularly (#24077) Fixes #22580 Co-authored-by: Felix Lange --- core/rawdb/accessors_metadata.go | 22 ++++++ eth/backend.go | 28 +++---- internal/shutdowncheck/shutdown_tracker.go | 85 ++++++++++++++++++++++ les/client.go | 49 ++++++------- 4 files changed, 145 insertions(+), 39 deletions(-) create mode 100644 internal/shutdowncheck/shutdown_tracker.go diff --git a/core/rawdb/accessors_metadata.go b/core/rawdb/accessors_metadata.go index 4c72ca7148..3b0fcf0f2d 100644 --- a/core/rawdb/accessors_metadata.go +++ b/core/rawdb/accessors_metadata.go @@ -139,6 +139,28 @@ func PopUncleanShutdownMarker(db ethdb.KeyValueStore) { } } +// UpdateUncleanShutdownMarker updates the last marker's timestamp to now. +func UpdateUncleanShutdownMarker(db ethdb.KeyValueStore) { + var uncleanShutdowns crashList + // Read old data + if data, err := db.Get(uncleanShutdownKey); err != nil { + log.Warn("Error reading unclean shutdown markers", "error", err) + } else if err := rlp.DecodeBytes(data, &uncleanShutdowns); err != nil { + log.Warn("Error decoding unclean shutdown markers", "error", err) + } + // This shouldn't happen because we push a marker on Backend instantiation + count := len(uncleanShutdowns.Recent) + if count == 0 { + log.Warn("No unclean shutdown marker to update") + return + } + uncleanShutdowns.Recent[count-1] = uint64(time.Now().Unix()) + data, _ := rlp.EncodeToBytes(uncleanShutdowns) + if err := db.Put(uncleanShutdownKey, data); err != nil { + log.Warn("Failed to write unclean-shutdown marker", "err", err) + } +} + // ReadTransitionStatus retrieves the eth2 transition status from the database func ReadTransitionStatus(db ethdb.KeyValueReader) []byte { data, _ := db.Get(transitionStatusKey) diff --git a/eth/backend.go b/eth/backend.go index 8e2bdf3642..a53982166d 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -47,6 +47,7 @@ import ( "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/internal/ethapi" + "github.com/ethereum/go-ethereum/internal/shutdowncheck" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/miner" "github.com/ethereum/go-ethereum/node" @@ -97,6 +98,8 @@ type Ethereum struct { p2pServer *p2p.Server lock sync.RWMutex // Protects the variadic fields (e.g. gas price and etherbase) + + shutdownTracker *shutdowncheck.ShutdownTracker // Tracks if and when the node has shutdown ungracefully } // New creates a new Ethereum object (including the @@ -157,6 +160,7 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) { bloomRequests: make(chan chan *bloombits.Retrieval), bloomIndexer: core.NewBloomIndexer(chainDb, params.BloomBitsBlocks, params.BloomConfirms), p2pServer: stack.Server(), + shutdownTracker: shutdowncheck.NewShutdownTracker(chainDb), } bcVersion := rawdb.ReadDatabaseVersion(chainDb) @@ -262,19 +266,9 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) { stack.RegisterProtocols(eth.Protocols()) stack.RegisterLifecycle(eth) - // Check for unclean shutdown - if uncleanShutdowns, discards, err := rawdb.PushUncleanShutdownMarker(chainDb); err != nil { - log.Error("Could not update unclean-shutdown-marker list", "error", err) - } else { - if discards > 0 { - log.Warn("Old unclean shutdowns found", "count", discards) - } - for _, tstamp := range uncleanShutdowns { - t := time.Unix(int64(tstamp), 0) - log.Warn("Unclean shutdown detected", "booted", t, - "age", common.PrettyAge(t)) - } - } + // Successful startup; push a marker and check previous unclean shutdowns. + eth.shutdownTracker.MarkStartup() + return eth, nil } @@ -549,6 +543,9 @@ func (s *Ethereum) Start() error { // Start the bloom bits servicing goroutines s.startBloomHandlers(params.BloomBitsBlocks) + // Regularly update shutdown marker + s.shutdownTracker.Start() + // Figure out a max peers count based on the server limits maxPeers := s.p2pServer.MaxPeers if s.config.LightServ > 0 { @@ -577,7 +574,10 @@ func (s *Ethereum) Stop() error { s.miner.Close() s.blockchain.Stop() s.engine.Close() - rawdb.PopUncleanShutdownMarker(s.chainDb) + + // Clean shutdown marker as the last thing before closing db + s.shutdownTracker.Stop() + s.chainDb.Close() s.eventMux.Stop() diff --git a/internal/shutdowncheck/shutdown_tracker.go b/internal/shutdowncheck/shutdown_tracker.go new file mode 100644 index 0000000000..c95b4f02f4 --- /dev/null +++ b/internal/shutdowncheck/shutdown_tracker.go @@ -0,0 +1,85 @@ +// Copyright 2021 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package shutdowncheck + +import ( + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/log" +) + +// ShutdownTracker is a service that reports previous unclean shutdowns +// upon start. It needs to be started after a successful start-up and stopped +// after a successful shutdown, just before the db is closed. +type ShutdownTracker struct { + db ethdb.Database + stopCh chan struct{} +} + +// NewShutdownTracker creates a new ShutdownTracker instance and has +// no other side-effect. +func NewShutdownTracker(db ethdb.Database) *ShutdownTracker { + return &ShutdownTracker{ + db: db, + stopCh: make(chan struct{}), + } +} + +// MarkStartup is to be called in the beginning when the node starts. It will: +// - Push a new startup marker to the db +// - Report previous unclean shutdowns +func (t *ShutdownTracker) MarkStartup() { + if uncleanShutdowns, discards, err := rawdb.PushUncleanShutdownMarker(t.db); err != nil { + log.Error("Could not update unclean-shutdown-marker list", "error", err) + } else { + if discards > 0 { + log.Warn("Old unclean shutdowns found", "count", discards) + } + for _, tstamp := range uncleanShutdowns { + t := time.Unix(int64(tstamp), 0) + log.Warn("Unclean shutdown detected", "booted", t, + "age", common.PrettyAge(t)) + } + } +} + +// Start runs an event loop that updates the current marker's timestamp every 5 minutes. +func (t *ShutdownTracker) Start() { + go func() { + ticker := time.NewTicker(5 * time.Minute) + defer ticker.Stop() + for { + select { + case <-ticker.C: + rawdb.UpdateUncleanShutdownMarker(t.db) + case <-t.stopCh: + return + } + } + }() +} + +// Stop will stop the update loop and clear the current marker. +func (t *ShutdownTracker) Stop() { + // Stop update loop. + t.stopCh <- struct{}{} + // Clear last marker. + rawdb.PopUncleanShutdownMarker(t.db) +} diff --git a/les/client.go b/les/client.go index c20c334395..43207f3443 100644 --- a/les/client.go +++ b/les/client.go @@ -35,6 +35,7 @@ import ( "github.com/ethereum/go-ethereum/eth/gasprice" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/internal/ethapi" + "github.com/ethereum/go-ethereum/internal/shutdowncheck" "github.com/ethereum/go-ethereum/les/downloader" "github.com/ethereum/go-ethereum/les/vflux" vfc "github.com/ethereum/go-ethereum/les/vflux/client" @@ -77,6 +78,8 @@ type LightEthereum struct { p2pServer *p2p.Server p2pConfig *p2p.Config udpEnabled bool + + shutdownTracker *shutdowncheck.ShutdownTracker // Tracks if and when the node has shutdown ungracefully } // New creates an instance of the light client. @@ -107,17 +110,18 @@ func New(stack *node.Node, config *ethconfig.Config) (*LightEthereum, error) { lesDb: lesDb, closeCh: make(chan struct{}), }, - peers: peers, - eventMux: stack.EventMux(), - reqDist: newRequestDistributor(peers, &mclock.System{}), - accountManager: stack.AccountManager(), - merger: merger, - engine: ethconfig.CreateConsensusEngine(stack, chainConfig, &config.Ethash, nil, false, chainDb), - bloomRequests: make(chan chan *bloombits.Retrieval), - bloomIndexer: core.NewBloomIndexer(chainDb, params.BloomBitsBlocksClient, params.HelperTrieConfirmations), - p2pServer: stack.Server(), - p2pConfig: &stack.Config().P2P, - udpEnabled: stack.Config().P2P.DiscoveryV5, + peers: peers, + eventMux: stack.EventMux(), + reqDist: newRequestDistributor(peers, &mclock.System{}), + accountManager: stack.AccountManager(), + merger: merger, + engine: ethconfig.CreateConsensusEngine(stack, chainConfig, &config.Ethash, nil, false, chainDb), + bloomRequests: make(chan chan *bloombits.Retrieval), + bloomIndexer: core.NewBloomIndexer(chainDb, params.BloomBitsBlocksClient, params.HelperTrieConfirmations), + p2pServer: stack.Server(), + p2pConfig: &stack.Config().P2P, + udpEnabled: stack.Config().P2P.DiscoveryV5, + shutdownTracker: shutdowncheck.NewShutdownTracker(chainDb), } var prenegQuery vfc.QueryFunc @@ -185,19 +189,9 @@ func New(stack *node.Node, config *ethconfig.Config) (*LightEthereum, error) { stack.RegisterProtocols(leth.Protocols()) stack.RegisterLifecycle(leth) - // Check for unclean shutdown - if uncleanShutdowns, discards, err := rawdb.PushUncleanShutdownMarker(chainDb); err != nil { - log.Error("Could not update unclean-shutdown-marker list", "error", err) - } else { - if discards > 0 { - log.Warn("Old unclean shutdowns found", "count", discards) - } - for _, tstamp := range uncleanShutdowns { - t := time.Unix(int64(tstamp), 0) - log.Warn("Unclean shutdown detected", "booted", t, - "age", common.PrettyAge(t)) - } - } + // Successful startup; push a marker and check previous unclean shutdowns. + leth.shutdownTracker.MarkStartup() + return leth, nil } @@ -352,6 +346,9 @@ func (s *LightEthereum) Protocols() []p2p.Protocol { func (s *LightEthereum) Start() error { log.Warn("Light client mode is an experimental feature") + // Regularly update shutdown marker + s.shutdownTracker.Start() + if s.udpEnabled && s.p2pServer.DiscV5 == nil { s.udpEnabled = false log.Error("Discovery v5 is not initialized") @@ -387,7 +384,9 @@ func (s *LightEthereum) Stop() error { s.engine.Close() s.pruner.close() s.eventMux.Stop() - rawdb.PopUncleanShutdownMarker(s.chainDb) + // Clean shutdown marker as the last thing before closing db + s.shutdownTracker.Stop() + s.chainDb.Close() s.lesDb.Close() s.wg.Wait()