close database on shutdown (#1403)

* bump avago to v1.12.0

* close database on shutdown

* update release notes

* rename verdb

* close only if standalone db

* bump avago branch version

* bump avago

Author: ceyonur

RELEASES.md

@@ -2,6 +2,14 @@
 
 ## Pending Release
 
+## Updates
+
+- Changed default write option from `Sync` to `NoSync` in PebbleDB
+
+## Fixes
+
+- Fixed database close on shutdown
+
 ## [v0.6.11](https://github.com/ava-labs/subnet-evm/releases/tag/v0.6.11)
 
 This release focuses on Standalone DB and database configs.

go.mod

@@ -5,7 +5,7 @@ go 1.22.8
 require (
 	github.com/VictoriaMetrics/fastcache v1.12.1
 	github.com/antithesishq/antithesis-sdk-go v0.3.8
-	github.com/ava-labs/avalanchego v1.12.0
+	github.com/ava-labs/avalanchego v1.12.0-config-pebble-sync
 	github.com/cespare/cp v0.1.0
 	github.com/crate-crypto/go-ipa v0.0.0-20231025140028-3c0104f4b233
 	github.com/davecgh/go-spew v1.1.1

go.sum

@@ -60,8 +60,8 @@ github.com/antithesishq/antithesis-sdk-go v0.3.8/go.mod h1:IUpT2DPAKh6i/YhSbt6Gl
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
-github.com/ava-labs/avalanchego v1.12.0 h1:NBx0vSOY1dCT0PeJzojIhNhx0NMQNem4GgTEN+v8Sx4=
-github.com/ava-labs/avalanchego v1.12.0/go.mod h1:yhD5dpZyStIVbxQ550EDi5w5SL7DQ/xGE6TIxosb7U0=
+github.com/ava-labs/avalanchego v1.12.0-config-pebble-sync h1:HvtadR1VLxsOe7Y2PSnNmLUXPvp1RnTgeWtk5cOuGYA=
+github.com/ava-labs/avalanchego v1.12.0-config-pebble-sync/go.mod h1:yhD5dpZyStIVbxQ550EDi5w5SL7DQ/xGE6TIxosb7U0=
 github.com/ava-labs/coreth v0.13.9-rc.1 h1:qIICpC/OZGYUP37QnLgIqqwGmxnLwLpZaUlqJNI85vU=
 github.com/ava-labs/coreth v0.13.9-rc.1/go.mod h1:7aMsRIo/3GBE44qWZMjnfqdqfcfZ5yShTTm2LObLaYo=
 github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g=

plugin/evm/block.go

@@ -55,7 +55,7 @@ func (b *Block) Accept(context.Context) error {
 
 	// Although returning an error from Accept is considered fatal, it is good
 	// practice to cleanup the batch we were modifying in the case of an error.
-	defer vm.db.Abort()
+	defer vm.versiondb.Abort()
 
 	log.Debug(fmt.Sprintf("Accepting block %s (%s) at height %d", b.ID().Hex(), b.ID(), b.Height()))
 
@@ -74,7 +74,7 @@ func (b *Block) Accept(context.Context) error {
 		return fmt.Errorf("failed to put %s as the last accepted block: %w", b.ID(), err)
 	}
 
-	return b.vm.db.Commit()
+	return b.vm.versiondb.Commit()
 }
 
 // handlePrecompileAccept calls Accept on any logs generated with an active precompile address that implements

plugin/evm/database/database.go

@@ -4,6 +4,7 @@
 package database
 
 import (
+	"encoding/json"
 	"fmt"
 	"path/filepath"
 
@@ -16,6 +17,7 @@ import (
 	"github.com/ava-labs/avalanchego/database/versiondb"
 	avalanchenode "github.com/ava-labs/avalanchego/node"
 	"github.com/ava-labs/avalanchego/utils/logging"
+	"github.com/prometheus/client_golang/prometheus"
 )
 
 const (
@@ -44,7 +46,7 @@ func NewStandaloneDatabase(dbConfig avalanchenode.DatabaseConfig, gatherer metri
 		db = memdb.New()
 	case pebbledb.Name:
 		dbPath := filepath.Join(dbConfig.Path, pebbledb.Name)
-		db, err = pebbledb.New(dbPath, dbConfig.Config, logger, dbRegisterer)
+		db, err = NewPebbleDB(dbPath, dbConfig.Config, logger, dbRegisterer)
 		if err != nil {
 			return nil, fmt.Errorf("couldn't create %s at %s: %w", pebbledb.Name, dbPath, err)
 		}
@@ -77,3 +79,20 @@ func NewStandaloneDatabase(dbConfig avalanchenode.DatabaseConfig, gatherer metri
 
 	return db, nil
 }
+
+func NewPebbleDB(file string, configBytes []byte, log logging.Logger, dbRegisterer prometheus.Registerer) (database.Database, error) {
+	cfg := pebbledb.DefaultConfig
+	// Use no sync for pebble db
+	cfg.Sync = false
+	if len(configBytes) > 0 {
+		if err := json.Unmarshal(configBytes, &cfg); err != nil {
+			return nil, err
+		}
+	}
+	// Marshal the config back to bytes to ensure that new defaults are applied
+	newCfgBytes, err := json.Marshal(cfg)
+	if err != nil {
+		return nil, err
+	}
+	return pebbledb.New(file, newCfgBytes, log, dbRegisterer)
+}

plugin/evm/syncervm_test.go

@@ -431,7 +431,7 @@ func testSyncerVM(t *testing.T, vmSetup *syncVMSetup, test syncTest) {
 	if test.expectedErr != nil {
 		require.ErrorIs(err, test.expectedErr)
 		// Note we re-open the database here to avoid a closed error when the test is for a shutdown VM.
-		chaindb := database.WrapDatabase(prefixdb.NewNested(ethDBPrefix, syncerVM.db))
+		chaindb := database.WrapDatabase(prefixdb.NewNested(ethDBPrefix, syncerVM.versiondb))
 		assertSyncPerformedHeights(t, chaindb, map[uint64]struct{}{})
 		return
 	}

plugin/evm/vm.go

@@ -195,15 +195,20 @@ type VM struct {
 	blockChain *core.BlockChain
 	miner      *miner.Miner
 
-	// [db] is the VM's current database managed by ChainState
-	db *versiondb.Database
+	// [versiondb] is the VM's current versioned database
+	versiondb *versiondb.Database
+
+	// [db] is the VM's current database
+	db database.Database
 
 	// metadataDB is used to store one off keys.
 	metadataDB database.Database
 
 	// [chaindb] is the database supplied to the Ethereum backend
 	chaindb ethdb.Database
 
+	usingStandaloneDB bool
+
 	// [acceptedBlockDB] is the database to store the last accepted
 	// block.
 	acceptedBlockDB database.Database
@@ -630,7 +635,7 @@ func (vm *VM) initializeStateSyncClient(lastAcceptedHeight uint64) error {
 		chaindb:              vm.chaindb,
 		metadataDB:           vm.metadataDB,
 		acceptedBlockDB:      vm.acceptedBlockDB,
-		db:                   vm.db,
+		db:                   vm.versiondb,
 		toEngine:             vm.toEngine,
 	})
 
@@ -880,6 +885,13 @@ func (vm *VM) Shutdown(context.Context) error {
 	}
 	vm.eth.Stop()
 	log.Info("Ethereum backend stop completed")
+	if vm.usingStandaloneDB {
+		if err := vm.db.Close(); err != nil {
+			log.Error("failed to close database: %w", err)
+		} else {
+			log.Info("Database closed")
+		}
+	}
 	vm.shutdownWg.Wait()
 	log.Info("Subnet-EVM Shutdown completed")
 	return nil

plugin/evm/vm_database.go

@@ -46,14 +46,16 @@ func (vm *VM) initializeDBs(avaDB avalanchedatabase.Database) error {
 			if err != nil {
 				return err
 			}
+			vm.usingStandaloneDB = true
 		}
 	}
 	// Use NewNested rather than New so that the structure of the database
 	// remains the same regardless of the provided baseDB type.
 	vm.chaindb = rawdb.NewDatabase(database.WrapDatabase(prefixdb.NewNested(ethDBPrefix, db)))
-	vm.db = versiondb.New(db)
-	vm.acceptedBlockDB = prefixdb.New(acceptedPrefix, vm.db)
-	vm.metadataDB = prefixdb.New(metadataPrefix, vm.db)
+	vm.versiondb = versiondb.New(db)
+	vm.acceptedBlockDB = prefixdb.New(acceptedPrefix, vm.versiondb)
+	vm.metadataDB = prefixdb.New(metadataPrefix, vm.versiondb)
+	vm.db = db
 	// Note warpDB and validatorsDB are not part of versiondb because it is not necessary
 	// that they are committed to the database atomically with
 	// the last accepted block.

scripts/versions.sh

@@ -4,5 +4,5 @@
 # shellcheck disable=SC2034
 
 # Don't export them as they're used in the context of other calls
-AVALANCHE_VERSION=${AVALANCHE_VERSION:-'v1.12.0'}
+AVALANCHE_VERSION=${AVALANCHE_VERSION:-'v1.12.0-config-pebble-sync'}
 GINKGO_VERSION=${GINKGO_VERSION:-'v2.2.0'}