feat(types): Deprecate the DBBackend variable in favor of new app-db-backend config entry (#11188)

* [10948]: Add changelog entry.

* [10948]: Deprecate the types.DBBackend variable and the NewLevelDB function. Create a NewDB function to replace them.

* [10948]: Add a DBBackend string to the simulation config and a flag for setting it. Update the simulation setup to use that instead of the compile-time DBBackend variable.

* [10948]: Update the mock app creator to use the NewDB function. Not sure what to do about the db backend in that case though.

* [10948]: Update changelog to reflect new db-backend field name.

* [10948]: Use the tendermint db-backend type for the snapshot db.

* [10948]: Update the last use of NewLevelDB by adding a parameter to openDB and uppdating calls to that to provide the db type to use.

* [10948]: Upddate the NewDB function to also have a default db backend type if an empty string is provided there.

* [10948]: Remove the new TODO in mock.NewApp. After looking through it's uses, there doesn't seem to be any desire to change it, and there's no easy way to communicate it.

* [10948]: Enhance the NewDB defer function to also add info to any err that is being returned.

* [10948]: Add some unit tests for NewDB.

* [10948]: Lint fixes.

* [10948]: Add a changelog entry to the deprecated section.

* [10948]: Update the makefile to no longer set the types.DBBackend value.

* [10948]: Use memdb for the mock app instead of goleveldb. I know it was a goleveldb before, but for a mock app, a memdb feels like a better choice (assuming 'mock' and 'mem' mean what I assume they mean).

* [10948]: Fix the store benchmark tests (had some index-out-of-range issues).

* [10948]: Fix cachekv store bench test calling iter.Key() before checking iter.Valid().

* [10948]: Remove the panic recovery from types.NewDB since dbm.NewDB returns an error now (it didn't originally, when NewLevelDB was first written).

* [10948]: Add changlog entry indicationg an API breaking change due to the DBBackend change.

* [10948]: Get rid of the types.NewDB function in favor of just using the tm-db version of it.

* [10948]: Fix Update the codeql-analysis github action to use go v1.17.

* [10948]: Add config file option for the app db backend type.

* [10948]: Adjust the comment on the app-db-backend config entry to clarify fallback behavior.

* [10948]: Add a default of GoLevelDBBackend to GetAppDBBackend. The old DBBackend variable defaulted to that, and some unit tests assume that behavior still exists.

* [10948]: Add the missing quotes around the app-db-backend value.

* [10948]: Small tweak to the changelog's deprecated entry.

* Add the go version declaration back into the codeql-analysis github action.

* [10948]: Update new use of openDB.

* [10948]: Put a brief delay after closing the test network. Hopefully that helps with address-in-use and non-empty directory errors.

Co-authored-by: Marko <marbar3778@yahoo.com>
This commit is contained in:
Daniel Wedul 2022-03-18 03:26:20 -06:00 committed by GitHub
parent 9e536388fb
commit 16e5d1a47b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 158 additions and 27 deletions

View File

@ -17,6 +17,9 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: 1.17
- uses: actions/setup-go@v3
with:

View File

@ -71,6 +71,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#11179](https://github.com/cosmos/cosmos-sdk/pull/11179) Add state rollback command.
* [\#10794](https://github.com/cosmos/cosmos-sdk/pull/10794) ADR-040: Add State Sync to V2 Store
* [\#11234](https://github.com/cosmos/cosmos-sdk/pull/11234) Add `GRPCClient` field to Client Context. If `GRPCClient` field is set to nil, the `Invoke` method would use ABCI query, otherwise use gprc.
* (types) [\#10948](https://github.com/cosmos/cosmos-sdk/issues/10948) Add `app-db-backend` to the `app.toml` config to replace the compile-time `types.DBbackend` variable.
### API Breaking Changes
@ -131,7 +132,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#10816](https://github.com/cosmos/cosmos-sdk/pull/10816) Reuse blocked addresses from the bank module. No need to pass them to distribution.
* [\#10852](https://github.com/cosmos/cosmos-sdk/pull/10852) Move `x/gov/types` to `x/gov/types/v1beta2`.
* [\#10922](https://github.com/cosmos/cosmos-sdk/pull/10922), [/#10957](https://github.com/cosmos/cosmos-sdk/pull/10957) Move key `server.Generate*` functions to testutil and support custom mnemonics in in-process testing network. Moved `TestMnemonic` from `testutil` package to `testdata`.
* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add safety check on bank module perms to allow module-specific mint restrictions (e.g. only minting a certain denom).* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add `bank.BaseKeeper.WithMintCoinsRestriction` function to restrict use of bank `MintCoins` usage.
* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add safety check on bank module perms to allow module-specific mint restrictions (e.g. only minting a certain denom).
* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add `bank.BaseKeeper.WithMintCoinsRestriction` function to restrict use of bank `MintCoins` usage.
* [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868), [\#10989](https://github.com/cosmos/cosmos-sdk/pull/10989) The Gov keeper accepts now 2 more mandatory arguments, the ServiceMsgRouter and a maximum proposal metadata length.
* [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868), [\#10989](https://github.com/cosmos/cosmos-sdk/pull/10989), [\#11093](https://github.com/cosmos/cosmos-sdk/pull/11093) The Gov keeper accepts now 2 more mandatory arguments, the ServiceMsgRouter and a gov Config including the max metadata length.
* [\#11124](https://github.com/cosmos/cosmos-sdk/pull/11124) Add `GetAllVersions` to application store
@ -250,6 +252,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Deprecated
* (x/upgrade) [\#9906](https://github.com/cosmos/cosmos-sdk/pull/9906) Deprecate `UpgradeConsensusState` gRPC query since this functionality is only used for IBC, which now has its own [IBC replacement](https://github.com/cosmos/ibc-go/blob/2c880a22e9f9cc75f62b527ca94aa75ce1106001/proto/ibc/core/client/v1/query.proto#L54)
* (types) [\#10948](https://github.com/cosmos/cosmos-sdk/issues/10948) Deprecate the types.DBBackend variable and types.NewLevelDB function. They are replaced by a new entry in `app.toml`: `app-db-backend` and `tendermint/tm-db`s `NewDB` function. If `app-db-backend` is defined, then it is used. Otherwise, if `types.DBBackend` is defined, it is used (until removed: [\#11241](https://github.com/cosmos/cosmos-sdk/issues/11241)). Otherwise, Tendermint config's `db-backend` is used.
## [v0.45.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.0) - 2022-01-18

View File

@ -46,10 +46,6 @@ ifeq ($(LEDGER_ENABLED),true)
endif
endif
ifeq (cleveldb,$(findstring cleveldb,$(COSMOS_BUILD_OPTIONS)))
build_tags += gcc
endif
ifeq (secp,$(findstring secp,$(COSMOS_BUILD_OPTIONS)))
build_tags += libsecp256k1_sdk
endif
@ -77,10 +73,9 @@ endif
# DB backend selection
ifeq (cleveldb,$(findstring cleveldb,$(COSMOS_BUILD_OPTIONS)))
ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=cleveldb
build_tags += gcc
endif
ifeq (badgerdb,$(findstring badgerdb,$(COSMOS_BUILD_OPTIONS)))
ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=badgerdb
BUILD_TAGS += badgerdb
endif
# handle rocksdb
@ -90,12 +85,10 @@ ifeq (rocksdb,$(findstring rocksdb,$(COSMOS_BUILD_OPTIONS)))
endif
CGO_ENABLED=1
BUILD_TAGS += rocksdb
ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=rocksdb
endif
# handle boltdb
ifeq (boltdb,$(findstring boltdb,$(COSMOS_BUILD_OPTIONS)))
BUILD_TAGS += boltdb
ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=boltdb
endif
ifeq (,$(findstring nostrip,$(COSMOS_BUILD_OPTIONS)))

View File

@ -70,6 +70,10 @@ type BaseConfig struct {
IndexEvents []string `mapstructure:"index-events"`
// IavlCacheSize set the size of the iavl tree cache.
IAVLCacheSize uint64 `mapstructure:"iavl-cache-size"`
// AppDBBackend defines the type of Database to use for the application and snapshots databases.
// An empty string indicates that the Tendermint config's DBBackend value should be used.
AppDBBackend string `mapstructure:"app-db-backend"`
}
// APIConfig defines the API listener configuration.
@ -210,6 +214,7 @@ func DefaultConfig() *Config {
MinRetainBlocks: 0,
IndexEvents: make([]string, 0),
IAVLCacheSize: 781250, // 50 MB
AppDBBackend: "",
},
Telemetry: telemetry.Config{
Enabled: false,
@ -269,6 +274,7 @@ func GetConfig(v *viper.Viper) Config {
IndexEvents: v.GetStringSlice("index-events"),
MinRetainBlocks: v.GetUint64("min-retain-blocks"),
IAVLCacheSize: v.GetUint64("iavl-cache-size"),
AppDBBackend: v.GetString("app-db-backend"),
},
Telemetry: telemetry.Config{
ServiceName: v.GetString("telemetry.service-name"),

View File

@ -75,6 +75,12 @@ index-events = [{{ range .BaseConfig.IndexEvents }}{{ printf "%q, " . }}{{end}}]
# Default cache size is 50mb.
iavl-cache-size = {{ .BaseConfig.IAVLCacheSize }}
# AppDBBackend defines the database backend type to use for the application and snapshots DBs.
# An empty string indicates that a fallback will be used.
# First fallback is the deprecated compile-time types.DBBackend value.
# Second fallback (if the types.DBBackend also isn't set), is the db-backend value set in Tendermint's config.toml.
app-db-backend = "{{ .BaseConfig.AppDBBackend }}"
###############################################################################
### Telemetry Configuration ###
###############################################################################

View File

@ -5,11 +5,13 @@ import (
"testing"
"github.com/stretchr/testify/require"
dbm "github.com/tendermint/tm-db"
)
func Test_openDB(t *testing.T) {
t.Parallel()
_, err := openDB(t.TempDir())
_, err := openDB(t.TempDir(), dbm.GoLevelDBBackend)
require.NoError(t, err)
}

View File

@ -37,7 +37,7 @@ func ExportCmd(appExporter types.AppExporter, defaultNodeHome string) *cobra.Com
return err
}
db, err := openDB(config.RootDir)
db, err := openDB(config.RootDir, GetAppDBBackend(serverCtx.Viper))
if err != nil {
return err
}

View File

@ -9,6 +9,7 @@ import (
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/log"
"github.com/tendermint/tendermint/types"
dbm "github.com/tendermint/tm-db"
bam "github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
@ -34,7 +35,7 @@ func testTxHandler(options middleware.TxHandlerOptions) tx.Handler {
// similar to a real app. Make sure rootDir is empty before running the test,
// in order to guarantee consistent results
func NewApp(rootDir string, logger log.Logger) (abci.Application, error) {
db, err := sdk.NewLevelDB("mock", filepath.Join(rootDir, "data"))
db, err := dbm.NewDB("mock", dbm.MemDBBackend, filepath.Join(rootDir, "data"))
if err != nil {
return nil, err
}

View File

@ -26,7 +26,7 @@ application.
ctx := GetServerContextFromCmd(cmd)
cfg := ctx.Config
home := cfg.RootDir
db, err := openDB(home)
db, err := openDB(home, GetAppDBBackend(ctx.Viper))
if err != nil {
return err
}

View File

@ -171,7 +171,7 @@ func startStandAlone(ctx *Context, appCreator types.AppCreator) error {
transport := ctx.Viper.GetString(flagTransport)
home := ctx.Viper.GetString(flags.FlagHome)
db, err := openDB(home)
db, err := openDB(home, GetAppDBBackend(ctx.Viper))
if err != nil {
return err
}
@ -230,12 +230,12 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App
}
}
traceWriterFile := ctx.Viper.GetString(flagTraceStore)
db, err := openDB(home)
db, err := openDB(home, GetAppDBBackend(ctx.Viper))
if err != nil {
return err
}
traceWriterFile := ctx.Viper.GetString(flagTraceStore)
traceWriter, err := openTraceWriter(traceWriterFile)
if err != nil {
return err

View File

@ -16,6 +16,7 @@ import (
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
"github.com/spf13/cast"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/spf13/viper"
@ -348,6 +349,21 @@ func WaitForQuitSignals() ErrorCode {
return ErrorCode{Code: int(sig.(syscall.Signal)) + 128}
}
// GetAppDBBackend gets the backend type to use for the application DBs.
func GetAppDBBackend(opts types.AppOptions) dbm.BackendType {
rv := cast.ToString(opts.Get("app-db-backend"))
if len(rv) == 0 {
rv = sdk.DBBackend
}
if len(rv) == 0 {
rv = cast.ToString(opts.Get("db-backend"))
}
if len(rv) != 0 {
return dbm.BackendType(rv)
}
return dbm.GoLevelDBBackend
}
func skipInterface(iface net.Interface) bool {
if iface.Flags&net.FlagUp == 0 {
return true // interface down
@ -372,9 +388,9 @@ func addrToIP(addr net.Addr) net.IP {
return ip
}
func openDB(rootDir string) (dbm.DB, error) {
func openDB(rootDir string, backendType dbm.BackendType) (dbm.DB, error) {
dataDir := filepath.Join(rootDir, "data")
return sdk.NewLevelDB("application", dataDir)
return dbm.NewDB("application", backendType, dataDir)
}
func openTraceWriter(traceWriterFile string) (w io.Writer, err error) {

View File

@ -11,14 +11,18 @@ import (
"testing"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
tmcfg "github.com/tendermint/tendermint/config"
dbm "github.com/tendermint/tm-db"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/server"
"github.com/cosmos/cosmos-sdk/server/config"
servertypes "github.com/cosmos/cosmos-sdk/server/types"
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
genutilcli "github.com/cosmos/cosmos-sdk/x/genutil/client/cli"
)
@ -438,3 +442,84 @@ func TestEmptyMinGasPrices(t *testing.T) {
err = cmd.ExecuteContext(ctx)
require.Errorf(t, err, sdkerrors.ErrAppConfig.Error())
}
type mapGetter map[string]interface{}
func (m mapGetter) Get(key string) interface{} {
return m[key]
}
var _ servertypes.AppOptions = mapGetter{}
func TestGetAppDBBackend(t *testing.T) {
origDBBackend := types.DBBackend
defer func() {
types.DBBackend = origDBBackend
}()
tests := []struct {
name string
dbBack string
opts mapGetter
exp dbm.BackendType
}{
{
name: "nothing set",
dbBack: "",
opts: mapGetter{},
exp: dbm.GoLevelDBBackend,
},
{
name: "only db-backend set",
dbBack: "",
opts: mapGetter{"db-backend": "db-backend value 1"},
exp: dbm.BackendType("db-backend value 1"),
},
{
name: "only DBBackend set",
dbBack: "DBBackend value 2",
opts: mapGetter{},
exp: dbm.BackendType("DBBackend value 2"),
},
{
name: "only app-db-backend set",
dbBack: "",
opts: mapGetter{"app-db-backend": "app-db-backend value 3"},
exp: dbm.BackendType("app-db-backend value 3"),
},
{
name: "app-db-backend and db-backend set",
dbBack: "",
opts: mapGetter{"db-backend": "db-backend value 4", "app-db-backend": "app-db-backend value 5"},
exp: dbm.BackendType("app-db-backend value 5"),
},
{
name: "app-db-backend and DBBackend set",
dbBack: "DBBackend value 6",
opts: mapGetter{"app-db-backend": "app-db-backend value 7"},
exp: dbm.BackendType("app-db-backend value 7"),
},
{
name: "db-backend and DBBackend set",
dbBack: "DBBackend value 8",
opts: mapGetter{"db-backend": "db-backend value 9"},
exp: dbm.BackendType("DBBackend value 8"),
},
{
name: "all of app-db-backend db-backend DBBackend set",
dbBack: "DBBackend value 10",
opts: mapGetter{"db-backend": "db-backend value 11", "app-db-backend": "app-db-backend value 12"},
exp: dbm.BackendType("app-db-backend value 12"),
},
}
for _, tc := range tests {
t.Run(tc.name, func(st *testing.T) {
types.DBBackend = tc.dbBack
act := server.GetAppDBBackend(tc.opts)
assert.Equal(st, tc.exp, act)
})
}
}

View File

@ -22,6 +22,7 @@ var (
FlagCommitValue bool
FlagOnOperationValue bool // TODO: Remove in favor of binary search for invariant violation
FlagAllInvariantsValue bool
FlagDBBackendValue string
FlagEnabledValue bool
FlagVerboseValue bool
@ -46,6 +47,7 @@ func GetSimulatorFlags() {
flag.BoolVar(&FlagCommitValue, "Commit", false, "have the simulation commit")
flag.BoolVar(&FlagOnOperationValue, "SimulateEveryOperation", false, "run slow invariants every operation")
flag.BoolVar(&FlagAllInvariantsValue, "PrintAllInvariants", false, "print all invariants if a broken invariant is found")
flag.StringVar(&FlagDBBackendValue, "DBBackend", "goleveldb", "custom db backend type")
// simulation flags
flag.BoolVar(&FlagEnabledValue, "Enabled", false, "enable the simulation")
@ -71,5 +73,6 @@ func NewConfigFromFlags() simulation.Config {
Commit: FlagCommitValue,
OnOperation: FlagOnOperationValue,
AllInvariants: FlagAllInvariantsValue,
DBBackend: FlagDBBackendValue,
}
}

View File

@ -261,7 +261,7 @@ func (a appCreator) newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, a
}
snapshotDir := filepath.Join(cast.ToString(appOpts.Get(flags.FlagHome)), "data", "snapshots")
snapshotDB, err := sdk.NewLevelDB("metadata", snapshotDir)
snapshotDB, err := dbm.NewDB("metadata", server.GetAppDBBackend(appOpts), snapshotDir)
if err != nil {
panic(err)
}

View File

@ -39,7 +39,7 @@ func SetupSimulation(dirPrefix, dbName string) (simtypes.Config, dbm.DB, string,
return simtypes.Config{}, nil, "", nil, false, err
}
db, err := sdk.NewLevelDB(dbName, dir)
db, err := dbm.NewDB(dbName, dbm.BackendType(config.DBBackend), dir)
if err != nil {
return simtypes.Config{}, nil, "", nil, false, err
}

View File

@ -35,7 +35,8 @@ func benchmarkBlankParentIteratorNext(b *testing.B, keysize int) {
iter := kvstore.Iterator(keys[0], keys[b.N])
defer iter.Close()
for _ = iter.Key(); iter.Valid(); iter.Next() {
for ; iter.Valid(); iter.Next() {
_ = iter.Key()
// deadcode elimination stub
sink = iter
}
@ -70,7 +71,8 @@ func benchmarkRandomSet(b *testing.B, keysize int) {
// Use a singleton for value, to not waste time computing it
value := randSlice(defaultValueSizeBz)
keys := generateRandomKeys(keysize, b.N)
// Add 1 to avoid issues when b.N = 1
keys := generateRandomKeys(keysize, b.N+1)
b.ReportAllocs()
b.ResetTimer()
@ -82,7 +84,8 @@ func benchmarkRandomSet(b *testing.B, keysize int) {
iter := kvstore.Iterator(keys[0], keys[b.N])
defer iter.Close()
for _ = iter.Key(); iter.Valid(); iter.Next() {
for ; iter.Valid(); iter.Next() {
_ = iter.Key()
// deadcode elimination stub
sink = iter
}
@ -100,7 +103,8 @@ func benchmarkIteratorOnParentWithManyDeletes(b *testing.B, numDeletes int) {
// Use simple values for keys, pick a random start,
// and take next D keys sequentially after.
startKey := randSlice(32)
keys := generateSequentialKeys(startKey, numDeletes)
// Add 1 to avoid issues when numDeletes = 1
keys := generateSequentialKeys(startKey, numDeletes+1)
// setup parent db with D keys.
for _, k := range keys {
mem.Set(k, value)
@ -118,10 +122,11 @@ func benchmarkIteratorOnParentWithManyDeletes(b *testing.B, numDeletes int) {
b.ReportAllocs()
b.ResetTimer()
iter := kvstore.Iterator(keys[0], keys[b.N])
iter := kvstore.Iterator(keys[0], keys[numDeletes])
defer iter.Close()
for _ = iter.Key(); iter.Valid(); iter.Next() {
for ; iter.Valid(); iter.Next() {
_ = iter.Key()
// deadcode elimination stub
sink = iter
}

View File

@ -603,6 +603,10 @@ func (n *Network) Cleanup() {
}
}
// Give a brief pause for things to finish closing in other processes. Hopefully this helps with the address-in-use errors.
// 100ms chosen randomly.
time.Sleep(100 * time.Millisecond)
if n.Config.CleanupDir {
_ = os.RemoveAll(n.BaseDir)
}

View File

@ -21,4 +21,6 @@ type Config struct {
OnOperation bool // run slow invariants every operation
AllInvariants bool // print all failed invariants if a broken invariant is found
DBBackend string // custom db backend type
}

View File

@ -11,7 +11,7 @@ import (
var (
// This is set at compile time. Could be cleveldb, defaults is goleveldb.
DBBackend = ""
DBBackend = "" // Deprecated: Use tendermint config's DBBackend value instead.
backend = dbm.GoLevelDBBackend
)
@ -85,6 +85,8 @@ func ParseTimeBytes(bz []byte) (time.Time, error) {
}
// NewLevelDB instantiate a new LevelDB instance according to DBBackend.
//
// Deprecated: Use NewDB (from "github.com/tendermint/tm-db") instead. Suggested backendType is tendermint config's DBBackend value.
func NewLevelDB(name, dir string) (db dbm.DB, err error) {
defer func() {
if r := recover(); r != nil {