eth, les: update unclean shutdown markers regularly (#24077)

Fixes #22580

Co-authored-by: Felix Lange <fjl@twurst.com>

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)

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
+	// Successful startup; push a marker and check previous unclean shutdowns.
-	if uncleanShutdowns, discards, err := rawdb.PushUncleanShutdownMarker(chainDb); err != nil {
+	eth.shutdownTracker.MarkStartup()
-		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))
-		}
-	}
 	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()
 

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 <http://www.gnu.org/licenses/>.
+
+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)
+}

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,
+		peers:           peers,
-		eventMux:       stack.EventMux(),
+		eventMux:        stack.EventMux(),
-		reqDist:        newRequestDistributor(peers, &mclock.System{}),
+		reqDist:         newRequestDistributor(peers, &mclock.System{}),
-		accountManager: stack.AccountManager(),
+		accountManager:  stack.AccountManager(),
-		merger:         merger,
+		merger:          merger,
-		engine:         ethconfig.CreateConsensusEngine(stack, chainConfig, &config.Ethash, nil, false, chainDb),
+		engine:          ethconfig.CreateConsensusEngine(stack, chainConfig, &config.Ethash, nil, false, chainDb),
-		bloomRequests:  make(chan chan *bloombits.Retrieval),
+		bloomRequests:   make(chan chan *bloombits.Retrieval),
-		bloomIndexer:   core.NewBloomIndexer(chainDb, params.BloomBitsBlocksClient, params.HelperTrieConfirmations),
+		bloomIndexer:    core.NewBloomIndexer(chainDb, params.BloomBitsBlocksClient, params.HelperTrieConfirmations),
-		p2pServer:      stack.Server(),
+		p2pServer:       stack.Server(),
-		p2pConfig:      &stack.Config().P2P,
+		p2pConfig:       &stack.Config().P2P,
-		udpEnabled:     stack.Config().P2P.DiscoveryV5,
+		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
+	// Successful startup; push a marker and check previous unclean shutdowns.
-	if uncleanShutdowns, discards, err := rawdb.PushUncleanShutdownMarker(chainDb); err != nil {
+	leth.shutdownTracker.MarkStartup()
-		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))
-		}
-	}
 	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()