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

[R4R]offline block prune #543

Merged
merged 26 commits into from
Jan 19, 2022
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e42f4e3
offline block prune
Mercybudda Nov 14, 2021
9271890
update
Mercybudda Dec 7, 2021
d4d8324
update
Mercybudda Dec 9, 2021
021aba5
update and add unit test
Mercybudda Dec 17, 2021
8790427
addressed comments from walt
Mercybudda Dec 22, 2021
fccd50e
Addressed comments from walt and Igor
Mercybudda Dec 25, 2021
be04eba
ensure MPT and snapshot matched
Mercybudda Dec 27, 2021
cb1ca71
add one more parameter to indicate blockprune
Mercybudda Dec 27, 2021
7b662d5
update the logic of creating freezerDb
Mercybudda Dec 27, 2021
eb1263d
update flag command description
Mercybudda Dec 28, 2021
d1fb290
expose the function for db inspect the offset/startBlockNumber
Mercybudda Dec 28, 2021
f532e37
add flags to inspect prune info
Mercybudda Dec 28, 2021
c14e873
rename flag of reserved-recent-blocks to block-amount-reserved
Mercybudda Dec 29, 2021
aaaee6b
addressed comments from walt
Mercybudda Dec 29, 2021
a2ed56c
handle the case of command interruption
Mercybudda Dec 31, 2021
6b4031a
refined goimports
Mercybudda Jan 4, 2022
8e051d4
addressed comments from walt
Mercybudda Jan 6, 2022
a516065
change the logic as restarting prune after interruption
Mercybudda Jan 6, 2022
de86417
addressed comments
Mercybudda Jan 11, 2022
f3e31a1
reclaimed freezer logic
Mercybudda Jan 11, 2022
69cdcfe
introduce flag to enable/disable check between MPT and snapshot
Mercybudda Jan 13, 2022
f9ea6a2
update the logic of frozen field in freezerDB
Mercybudda Jan 13, 2022
6e13383
update the code in all places related to freezer change
Mercybudda Jan 14, 2022
29279f2
addressed comments from dylan
Mercybudda Jan 18, 2022
dbfc231
update the logic for backup block difficulty
Mercybudda Jan 19, 2022
37f2e89
addressed comments from dylan
Mercybudda Jan 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update and add unit test
  • Loading branch information
Mercybudda committed Dec 29, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 021aba58ec74a43251215f0195015116a18ad517
151 changes: 151 additions & 0 deletions cmd/geth/pruneblock_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
// Copyright 2016 The go-ethereum Authors
// This file is part of go-ethereum.
//
// go-ethereum is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// go-ethereum 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 General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.

package main

import (
"io/ioutil"
"math/big"
"os"
"testing"

"github.com/ethereum/go-ethereum/cmd/utils"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/consensus"
"github.com/ethereum/go-ethereum/consensus/ethash"
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/rawdb"
"github.com/ethereum/go-ethereum/core/state/pruner"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/core/vm"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/eth"
"github.com/ethereum/go-ethereum/eth/ethconfig"
"github.com/ethereum/go-ethereum/ethdb"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/node"
"github.com/ethereum/go-ethereum/params"
)

// So we can deterministically seed different blockchains
var (
canonicalSeed = 1

testKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
// testAddr is the Ethereum address of the tester account.
testAddr = crypto.PubkeyToAddress(testKey.PublicKey)
)

func TestOfflineBlockPrune(t *testing.T) {
datadir, err := ioutil.TempDir("", "")
if err != nil {
t.Fatalf("Failed to create temporary datadir: %v", err)
}
os.RemoveAll(datadir)

chaindbPath := datadir + "/chaindata"
oldAncientPath := chaindbPath + "/ancient"
newAncientPath := chaindbPath + "/ancient_back"
//create a database with ancient freezer
db, err := rawdb.NewLevelDBDatabaseWithFreezer(chaindbPath, 512, utils.MakeDatabaseHandles(), oldAncientPath, "", false)

if err != nil {
t.Fatalf("failed to create database with ancient backend")
}

testBlockPruneForOffSetZero(t, true, db, newAncientPath, oldAncientPath, chaindbPath)

}

func testBlockPruneForOffSetZero(t *testing.T, full bool, db ethdb.Database, backFreezer, oldfreezer, chaindbPath string) error {
// Make chain starting from genesis
blockchain, n, err := newCanonical(t, db, ethash.NewFaker(), 92000, full, chaindbPath)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a block chain with empty blocks, it can not test prune block body, transaction receipt. A block chain with transactions is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

Choose a reason for hiding this comment

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

rename n to node

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if err != nil {
t.Fatalf("failed to make new canonical chain: %v", err)
}
defer blockchain.Stop()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since prune do not need blockchain to be alive, we can stop it rather than defer. No sure, but have a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


testBlockPruner, err := pruner.NewBlockPruner(db, n, oldfreezer)
if err != nil {
t.Fatalf("failed to make new blockpruner: %v", err)
}

db.Close()
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can delete db.Close() if we stop the blockchain before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if err := testBlockPruner.BlockPruneBackUp(chaindbPath, 512, utils.MakeDatabaseHandles(), backFreezer, oldfreezer, "", false); err != nil {
log.Error("Failed to back up block", "err", err)
return err
}

return nil

}

Copy link
Collaborator

Choose a reason for hiding this comment

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

The testcase is not enough. We need query header, receipt, block, transaction before prune, and do query again to verify we do the prune correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

// newCanonical creates a chain database, and injects a deterministic canonical
// chain. Depending on the full flag, if creates either a full block chain or a
// header only chain.
func newCanonical(t *testing.T, db ethdb.Database, engine consensus.Engine, height int, full bool, chaindbPath string) (*core.BlockChain, *node.Node, error) {

var (
key, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
address = crypto.PubkeyToAddress(key.PublicKey)
funds = big.NewInt(1000000000)
gspec = &core.Genesis{Config: params.TestChainConfig, Alloc: core.GenesisAlloc{address: {Balance: funds}}}
genesis = gspec.MustCommit(db)
)

// Initialize a fresh chain with only a genesis block
blockchain, _ := core.NewBlockChain(db, nil, params.AllEthashProtocolChanges, engine, vm.Config{}, nil, nil)

// Full block-chain requested, same to GenerateChain func
blocks := makeBlockChain(genesis, height, engine, db, canonicalSeed)
_, err := blockchain.InsertChain(blocks)
nd, _ := startEthService(t, gspec, blocks, chaindbPath)
return blockchain, nd, err
}

// makeBlockChain creates a deterministic chain of blocks rooted at parent.
func makeBlockChain(parent *types.Block, n int, engine consensus.Engine, db ethdb.Database, seed int) []*types.Block {
blocks, _ := core.GenerateChain(params.TestChainConfig, parent, engine, db, n, func(i int, b *core.BlockGen) {
b.SetCoinbase(common.Address{0: byte(seed), 19: byte(i)})
})
return blocks
}

// startEthService creates a full node instance for testing.
func startEthService(t *testing.T, genesis *core.Genesis, blocks []*types.Block, chaindbPath string) (*node.Node, *eth.Ethereum) {
t.Helper()

n, err := node.New(&node.Config{DataDir: chaindbPath})
if err != nil {
t.Fatal("can't create node:", err)
}

ethcfg := &ethconfig.Config{Genesis: genesis, Ethash: ethash.Config{PowMode: ethash.ModeFake}}
ethservice, err := eth.New(n, ethcfg)
if err != nil {
t.Fatal("can't create eth service:", err)
}
if err := n.Start(); err != nil {
t.Fatal("can't start node:", err)
}
if _, err := ethservice.BlockChain().InsertChain(blocks); err != nil {
n.Close()
t.Fatal("can't import test blocks:", err)
}
ethservice.SetEtherbase(testAddr)

return n, ethservice
}
9 changes: 7 additions & 2 deletions cmd/geth/snapshot.go
Original file line number Diff line number Diff line change
@@ -93,6 +93,7 @@ the trie clean cache with default directory will be deleted.
},
Description: `
Offline prune for block data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is too simple. Please describe it better.

Copy link
Contributor

Choose a reason for hiding this comment

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

and can you add the side effects if we prune the blocks like we can not query the history of blocks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

For AncientFlag, please specify the absolute path of node/geth/chaindata.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why it have to be absolute path?

`,
},
{
@@ -177,8 +178,10 @@ func pruneBlock(ctx *cli.Context) error {
newAncientPath = path + "/ancient_back"
} else {
utils.Fatalf("Prune failed, did not specify the AncientPath %v")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better have a default ancient path.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You have %v here , but no arg.

return errors.New("Prune failed, did not specify the AncientPath")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fatalf will do os.Exit(1), please just return, do not Fatalf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

//TODO for filelock
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will we have file lock?

//lock, _, err := fileutil.Flock(filepath.Join(oldAncientPath, "FLOCK"))
// lock, _, err := fileutil.Flock(oldAncientPath)
// if err != nil {
@@ -196,9 +199,10 @@ func pruneBlock(ctx *cli.Context) error {
case !filepath.IsAbs(oldAncientPath):
oldAncientPath = stack.ResolvePath(oldAncientPath)
}
pruner, err := pruner.NewBlockPruner(chaindb, stack, stack.ResolvePath(""), oldAncientPath)
pruner, err := pruner.NewBlockPruner(chaindb, stack, oldAncientPath)
if err != nil {
utils.Fatalf("Failed to create block pruner", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Get err arg here, but no %v

return err
}

if err := pruner.BlockPruneBackUp(name, config.Eth.DatabaseCache, utils.MakeDatabaseHandles(), newAncientPath, oldAncientPath, "", false); err != nil {
@@ -209,11 +213,12 @@ func pruneBlock(ctx *cli.Context) error {

log.Info("geth block offline pruning backup successfully")
Copy link
Collaborator

Choose a reason for hiding this comment

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

"geth block offline pruning backup successfully" ==> back up block successfully is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


//After backing up successfully, rename the new ancientdb name to the original one, and delete the old ancientdb
if err := pruner.BlockPrune(oldAncientPath, newAncientPath); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is quite weried you already pass oldAncientPath when NewBlockPruner, but pass it again when doing BlockPruneBackUp, it is duplicated.

Copy link
Collaborator

@unclezoro unclezoro Dec 23, 2021

Choose a reason for hiding this comment

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

It is really confusing between blockpruner.BlockPruneBackUp and pruner.BlockPrune. pruner.BlockPrune is doing dir rename, so try better name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

utils.Fatalf("Failed to prune block", err)
return err
}
//lock.Release()
//TODO lock.Release()
log.Info("Block prune successfully")
return nil
}
23 changes: 12 additions & 11 deletions core/rawdb/database.go
Original file line number Diff line number Diff line change
@@ -180,6 +180,8 @@ func NewDatabaseWithFreezer(db ethdb.KeyValueStore, freezer string, namespace st
// If the genesis hash is empty, we have a new key-value store, so nothing to
// validate in this method. If, however, the genesis hash is not nil, compare
// it to the freezer content.
// Only to check the followings when offset equal to 0, otherwise the block number
// in ancientdb did not start with 0, no genesis block in ancientdb as well.
if offset == 0 {
if kvgenesis, _ := db.Get(headerHashKey(0)); len(kvgenesis) > 0 {
Copy link
Contributor

@yutianwu yutianwu Dec 24, 2021

Choose a reason for hiding this comment

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

what if we change this to if kvgenesis, _ := db.Get(headerHashKey(0)); len(kvgenesis) > 0 && offset == 0 and remove the upper if statement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Done

if frozen, _ := frdb.Ancients(); frozen > 0 {
@@ -234,10 +236,9 @@ func NewDatabaseWithFreezer(db ethdb.KeyValueStore, freezer string, namespace st
}, nil
}

// NewDatabaseWithFreezer creates a high level database on top of a given key-
// value data store with a freezer moving immutable chain segments into cold
// storage.
//Without goroutine of freeze running
// NewDatabaseWithFreezerForPruneBlock creates or open if existed a high level database on top of a given key-
// value data store with a freezer, but without goroutine of freezer running to avoid the uncertainty
// between kvdb and freezer goroutine when open/close db.
func NewDatabaseWithFreezerForPruneBlock(db ethdb.KeyValueStore, freezer string, namespace string, readonly bool) (ethdb.Database, error) {
// Create the idle freezer instance
frdb, err := newFreezer(freezer, namespace, readonly)
@@ -251,9 +252,9 @@ func NewDatabaseWithFreezerForPruneBlock(db ethdb.KeyValueStore, freezer string,
}, nil
}

// NewDatabaseWithFreezer creates a high level database on top of a given key-
// value data store with a freezer moving immutable chain segments into cold
// storage.
// NewDatabaseWithFreezerBackup creates or open if existed a high level database on top of a given key-
// value data store with a freezer, passed the params of offset, without goroutine of freezer running
//to avoid the uncertainty between kvdb and freezer goroutine when open/close db
func NewDatabaseWithFreezerBackup(offset uint64, db ethdb.KeyValueStore, freezer string, namespace string, readonly bool) (ethdb.Database, error) {
// Create the idle freezer instance
frdb, err := newFreezer(freezer, namespace, readonly)
@@ -308,8 +309,8 @@ func NewLevelDBDatabaseWithFreezer(file string, cache int, handles int, freezer
return frdb, nil
}

// NewLevelDBDatabaseWithFreezer creates a persistent key-value database with a
// freezer moving immutable chain segments into cold storage.
// NewLevelDBDatabaseWithFreezerForPruneBlock creates a persistent key-value database with a
// freezer.
func NewLevelDBDatabaseWithFreezerForPruneBlock(file string, cache int, handles int, freezer string, namespace string, readonly bool) (ethdb.Database, error) {
kvdb, err := leveldb.New(file, cache, handles, namespace, readonly)
if err != nil {
@@ -323,8 +324,8 @@ func NewLevelDBDatabaseWithFreezerForPruneBlock(file string, cache int, handles
return frdb, nil
}

// NewLevelDBDatabaseWithFreezer creates a persistent key-value database with a
// freezer moving immutable chain segments into cold storage.
// NewLevelDBDatabaseWithFreezerBackup creates a persistent key-value database with a
// freezer.
func NewLevelDBDatabaseWithFreezerBackup(offset uint64, file string, cache int, handles int, freezer string, namespace string, readonly bool) (ethdb.Database, error) {
kvdb, err := leveldb.New(file, cache, handles, namespace, readonly)
if err != nil {
33 changes: 17 additions & 16 deletions core/state/pruner/pruner.go
Original file line number Diff line number Diff line change
@@ -90,7 +90,6 @@ type Pruner struct {

type BlockPruner struct {
db ethdb.Database
chaindbDir string
ancientdbDir string
n *node.Node
}
@@ -125,11 +124,10 @@ func NewPruner(db ethdb.Database, datadir, trieCachePath string, bloomSize, trie
}, nil
}

func NewBlockPruner(db ethdb.Database, n *node.Node, chaindbDir, ancientdbDir string) (*BlockPruner, error) {
func NewBlockPruner(db ethdb.Database, n *node.Node, ancientdbDir string) (*BlockPruner, error) {

return &BlockPruner{
db: db,
chaindbDir: chaindbDir,
ancientdbDir: ancientdbDir,
n: n,
}, nil
@@ -253,37 +251,39 @@ func prune(snaptree *snapshot.Tree, root common.Hash, maindb ethdb.Database, sta
return nil
}

// Prune block body data
// Backup the ancient data for the old ancient db, i.e. the most recent 128 blocks in ancient db.
func (p *BlockPruner) BlockPruneBackUp(name string, cache, handles int, backFreezer, freezer, namespace string, readonly bool) error {
//Back-up the necessary data within original ancient directory, create new freezer backup directory backFreezer

start := time.Now()

//write the latest 128 blocks data of the ancient db
// If we can't access the freezer or it's empty, abort
chainDb, err := p.n.OpenDatabaseWithFreezerForPruneBlock(name, cache, handles, freezer, namespace, readonly)
if err != nil {
log.Error("Failed to open ancient database: %v", err)
return err
}

oldOffSet := rawdb.ReadOffSetOfAncientFreezer(chainDb)
// Get the number of items in ancient db, frozen it is.
frozen, err := chainDb.Ancients()

// Write the latest 128 blocks data of the ancient db
// If we can't access the freezer or it's empty, abort.
if err != nil || frozen == 0 {
return errors.New("can't access the freezer or it's empty, abort")
}
startBlockNumber := frozen + oldOffSet - 128

// Get the actual start block number.
startBlockNumber := frozen - 128 + oldOffSet
Copy link
Collaborator

Choose a reason for hiding this comment

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

128 is a magic number here, please use constant value.

// For every round, newoffset actually equals to the startBlockNumber in ancient db.
newOffSet := oldOffSet + frozen - 128

//write the new offset into db for new freezer usage
// Write the new offset into db for the future new freezer usage.
rawdb.WriteOffSetOfAncientFreezer(chainDb, newOffSet)

// Initialize the slice to buffer the 128 block data.
blockList := make([]*types.Block, 0, 128)
receiptsList := make([]types.Receipts, 0, 128)
externTdList := make([]*big.Int, 0, 128)

//All ancient data within the most recent 128 blocks write into memory for future new ancient_back directory usage
// All ancient data within the most recent 128 blocks write into memory buffer for future new ancient_back directory usage.
for blockNumber := startBlockNumber; blockNumber < frozen+oldOffSet; blockNumber++ {
blockHash := rawdb.ReadCanonicalHash(chainDb, blockNumber)
block := rawdb.ReadBlock(chainDb, blockHash, blockNumber)
@@ -298,15 +298,16 @@ func (p *BlockPruner) BlockPruneBackUp(name string, cache, handles int, backFree
externTd := new(big.Int).Add(block.Difficulty(), td)
externTdList = append(externTdList, externTd)
}

chainDb.Close()

// Create new freezer backup directory backFreezer, in the db wrapper, using the same kv db but only change the ancient db, /chaindb/ancient_backup
chainDbBack, err := p.n.OpenDatabaseWithFreezerBackup(newOffSet, name, cache, handles, backFreezer, namespace, readonly)
if err != nil {
log.Error("Failed to open ancient database: %v", err)
return err
}
//Write into ancient_backup

// Write into ancient_backup
for id := 0; id < len(blockList); id++ {
rawdb.WriteAncientBlock(chainDbBack, blockList[id], receiptsList[id], externTdList[id])
}
@@ -317,13 +318,13 @@ func (p *BlockPruner) BlockPruneBackUp(name string, cache, handles int, backFree
}

func BlockPrune(oldAncientPath, newAncientPath string) error {
//Delete directly for the old ancientdb, e.g.: path ../chaindb/ancient
// Delete directly for the old ancientdb, e.g.: path ../chaindb/ancient
if err := os.RemoveAll(oldAncientPath); err != nil {
log.Error("Failed to remove old ancient directory %v", err)
return err
}

//Rename the new ancientdb path same to the old
// Rename the new ancientdb path same to the old
if err := os.Rename(newAncientPath, oldAncientPath); err != nil {
log.Error("Failed to rename new ancient directory")
return err
15 changes: 8 additions & 7 deletions node/node.go
Original file line number Diff line number Diff line change
@@ -634,11 +634,11 @@ func (n *Node) OpenDatabaseWithFreezer(name string, cache, handles int, freezer,
return db, err
}

// OpenDatabaseWithFreezer opens an existing database with the given name (or
// The big difference is that the freezer inside was not running to avoid the mess between kvdb and freezer
// while opening/closing db.
// OpenDatabaseWithFreezerForPruneBlock opens an existing database with the given name (or
// creates one if no previous can be found) from within the node's data directory,
// also attaching a chain freezer to it that moves ancient chain data from the
// database to immutable append-only files. If the node is an ephemeral one, a
// memory database is returned.
// also attaching a chain freezer to it. If the node is an ephemeral one, a memory database is returned.
func (n *Node) OpenDatabaseWithFreezerForPruneBlock(name string, cache, handles int, freezer, namespace string, readonly bool) (ethdb.Database, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please try not to invent new function, too many duplicated code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These new functions were the function without freeze go-routine running, to avoid the case that freeze go-routine did not stop while db closing.

n.lock.Lock()
defer n.lock.Unlock()
@@ -667,10 +667,11 @@ func (n *Node) OpenDatabaseWithFreezerForPruneBlock(name string, cache, handles
return db, err
}

// OpenDatabaseWithFreezer opens an existing database with the given name (or
// The big difference is that the freezer inside was not running to avoid the mess between kvdb and freezer
// while opening/closing db.
// OpenDatabaseWithFreezerBackup opens an existing database with the given name (or
// creates one if no previous can be found) from within the node's data directory,
// also attaching a chain freezer to it that moves ancient chain data from the
// database to immutable append-only files. If the node is an ephemeral one, a
// also attaching a chain freezer to it. If the node is an ephemeral one, a
// memory database is returned.
func (n *Node) OpenDatabaseWithFreezerBackup(offset uint64, name string, cache, handles int, freezer, namespace string, readonly bool) (ethdb.Database, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

n.lock.Lock()