diff --git a/CHANGELOG.md b/CHANGELOG.md index d14637a12c622..0236466440dc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,133 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#13881](https://github.com/cosmos/cosmos-sdk/pull/13881) Optimize iteration on nested cached KV stores and other operations in general. +<<<<<<< HEAD +======= +### State Machine Breaking + +* (x/group) [#13742](https://github.com/cosmos/cosmos-sdk/pull/13742) Migrate group policy account from module accounts to base account. +* (codec) [#13307](https://github.com/cosmos/cosmos-sdk/pull/13307) Register all modules' `Msg`s with group's ModuleCdc so that Amino sign bytes are correctly generated. +* (codec) [#13196](https://github.com/cosmos/cosmos-sdk/pull/13196) Register all modules' `Msg`s with gov's ModuleCdc so that Amino sign bytes are correctly generated. +* (group) [#13592](https://github.com/cosmos/cosmos-sdk/pull/13592) Fix group types registration with Amino. +* (x/distribution) [#12852](https://github.com/cosmos/cosmos-sdk/pull/12852) Deprecate `CommunityPoolSpendProposal`. Please execute a `MsgCommunityPoolSpend` message via the new v1 `x/gov` module instead. This message can be used to directly fund the `x/gov` module account. +* (x/bank) [#12610](https://github.com/cosmos/cosmos-sdk/pull/12610) `MsgMultiSend` now allows only a single input. +* (x/bank) [#12630](https://github.com/cosmos/cosmos-sdk/pull/12630) Migrate `x/bank` to self-managed parameters and deprecate its usage of `x/params`. +* (x/auth) [#12475](https://github.com/cosmos/cosmos-sdk/pull/12475) Migrate `x/auth` to self-managed parameters and deprecate its usage of `x/params`. +* (x/slashing) [#12399](https://github.com/cosmos/cosmos-sdk/pull/12399) Migrate `x/slashing` to self-managed parameters and deprecate its usage of `x/params`. +* (x/mint) [#12363](https://github.com/cosmos/cosmos-sdk/pull/12363) Migrate `x/mint` to self-managed parameters and deprecate it's usage of `x/params`. +* (x/distribution) [#12434](https://github.com/cosmos/cosmos-sdk/pull/12434) Migrate `x/distribution` to self-managed parameters and deprecate it's usage of `x/params`. +* (x/crisis) [#12445](https://github.com/cosmos/cosmos-sdk/pull/12445) Migrate `x/crisis` to self-managed parameters and deprecate it's usage of `x/params`. +* (x/gov) [#12631](https://github.com/cosmos/cosmos-sdk/pull/12631) Migrate `x/gov` to self-managed parameters and deprecate it's usage of `x/params`. +* (x/staking) [#12409](https://github.com/cosmos/cosmos-sdk/pull/12409) Migrate `x/staking` to self-managed parameters and deprecate it's usage of `x/params`. +* (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) Move the SendEnabled information out of the Params and into the state store directly. +* (x/gov) [#12771](https://github.com/cosmos/cosmos-sdk/pull/12771) Initial deposit requirement for proposals at submission time. +* (x/staking) [#12967](https://github.com/cosmos/cosmos-sdk/pull/12967) `unbond` now creates only one unbonding delegation entry when multiple unbondings exist at a single height (e.g. through multiple messages in a transaction). +* (x/auth/vesting) [#13502](https://github.com/cosmos/cosmos-sdk/pull/13502) Add Amino Msg registration for `MsgCreatePeriodicVestingAccount`. +* (x/auth)[#13780](https://github.com/cosmos/cosmos-sdk/pull/13780) `id` (type of int64) in `AccountAddressByID` grpc query is now deprecated, update to account-id(type of uint64) to use `AccountAddressByID`. +* (x/group) [#13876](https://github.com/cosmos/cosmos-sdk/pull/13876) Fix group MinExecutionPeriod that is checked on execution now, instead of voting period end. +* (store) [#14378](https://github.com/cosmos/cosmos-sdk/pull/14378) The `CacheKV` store is thread-safe again, which includes improved iteration and deletion logic. Iteration is on a strictly isolated view now, which is breaking from previous behavior. + +### API Breaking Changes + +* (crypto/keyring) [#14151](https://github.com/cosmos/cosmos-sdk/pull/14151) Move keys presentation from `crypto/keyring` to `client/keys` +* (modules) [#13850](https://github.com/cosmos/cosmos-sdk/pull/13850) and [#14046](https://github.com/cosmos/cosmos-sdk/pull/14046) Remove gogoproto stringer annotations. This removes the custom `String()` methods on all types that were using the annotations. +* (x/auth) [#13850](https://github.com/cosmos/cosmos-sdk/pull/13850/) Remove `MarshalYAML` methods from module (`x/...`) types. +* (x/auth) [#13877](https://github.com/cosmos/cosmos-sdk/pull/13877) Rename `AccountKeeper`'s `GetNextAccountNumber` to `NextAccountNumber`. +* (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) The `NewQueryEvidenceRequest` function now takes `hash` as a HEX encoded `string`. +* (server) [#13485](https://github.com/cosmos/cosmos-sdk/pull/13485) The `Application` service now requires the `RegisterNodeService` method to be implemented. +* [#13437](https://github.com/cosmos/cosmos-sdk/pull/13437) Add a list of modules to export argument in `ExportAppStateAndValidators`. +* (x/slashing) [#13427](https://github.com/cosmos/cosmos-sdk/pull/13427) Move `x/slashing/testslashing` to `x/slashing/testutil` for consistency with other modules. +* (x/staking) [#13427](https://github.com/cosmos/cosmos-sdk/pull/13427) Move `x/staking/teststaking` to `x/staking/testutil` for consistency with other modules. +* (simapp) [#13402](https://github.com/cosmos/cosmos-sdk/pull/13402) Move simulation flags to `x/simulation/client/cli`. +* (simapp) [#13402](https://github.com/cosmos/cosmos-sdk/pull/13402) Move simulation helpers functions (`SetupSimulation`, `SimulationOperations`, `CheckExportSimulation`, `PrintStats`, `GetSimulationLog`) to `testutil/sims`. +* (simapp) [#13402](https://github.com/cosmos/cosmos-sdk/pull/13402) Move `testutil/rest` package to `testutil`. +* (types) [#13380](https://github.com/cosmos/cosmos-sdk/pull/13380) Remove deprecated `sdk.NewLevelDB`. +* (simapp) [#13378](https://github.com/cosmos/cosmos-sdk/pull/13378) Move `simapp.App` to `runtime.AppI`. +* (tx) [#12659](https://github.com/cosmos/cosmos-sdk/pull/12659) Remove broadcast mode `block`. +* (db) [#13370](https://github.com/cosmos/cosmos-sdk/pull/13370) remove storev2alpha1, see also https://github.com/cosmos/cosmos-sdk/pull/13371 +* (x/bank) [#12706](https://github.com/cosmos/cosmos-sdk/pull/12706) Removed the `testutil` package from the `x/bank/client` package. +* (simapp) [#12747](https://github.com/cosmos/cosmos-sdk/pull/12747) Remove `simapp.MakeTestEncodingConfig`. Please use `moduletestutil.MakeTestEncodingConfig` (`types/module/testutil`) in tests instead. +* (x/bank) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) `NewSendAuthorization` takes a new argument of an optional list of addresses allowed to receive bank assests via authz MsgSend grant. You can pass `nil` for the same behavior as before, i.e. any recipient is allowed. +* (x/bank) [#12593](https://github.com/cosmos/cosmos-sdk/pull/12593) Add `SpendableCoin` method to `BaseViewKeeper` +* (x/slashing) [#12581](https://github.com/cosmos/cosmos-sdk/pull/12581) Remove `x/slashing` legacy querier. +* (types) [#12355](https://github.com/cosmos/cosmos-sdk/pull/12355) Remove the compile-time `types.DBbackend` variable. Removes usage of the same in server/util.go +* (x/gov) [#12368](https://github.com/cosmos/cosmos-sdk/pull/12369) Gov keeper is now passed by reference instead of copy to make post-construction mutation of Hooks and Proposal Handlers possible at a framework level. +* (simapp) [#12270](https://github.com/cosmos/cosmos-sdk/pull/12270) Remove `invCheckPeriod uint` attribute from `SimApp` struct as per migration of `x/crisis` to app wiring +* (simapp) [#12334](https://github.com/cosmos/cosmos-sdk/pull/12334) Move `simapp.ConvertAddrsToValAddrs` and `simapp.CreateTestPubKeys ` to respectively `simtestutil.ConvertAddrsToValAddrs` and `simtestutil.CreateTestPubKeys` (`testutil/sims`) +* (simapp) [#12312](https://github.com/cosmos/cosmos-sdk/pull/12312) Move `simapp.EmptyAppOptions` to `simtestutil.EmptyAppOptions` (`testutil/sims`) +* (simapp) [#12312](https://github.com/cosmos/cosmos-sdk/pull/12312) Remove `skipUpgradeHeights map[int64]bool` and `homePath string` from `NewSimApp` constructor as per migration of `x/upgrade` to app-wiring. +* (testutil) [#12278](https://github.com/cosmos/cosmos-sdk/pull/12278) Move all functions from `simapp/helpers` to `testutil/sims` +* (testutil) [#12233](https://github.com/cosmos/cosmos-sdk/pull/12233) Move `simapp.TestAddr` to `simtestutil.TestAddr` (`testutil/sims`) +* (x/staking) [#12102](https://github.com/cosmos/cosmos-sdk/pull/12102) Staking keeper now is passed by reference instead of copy. Keeper's SetHooks no longer returns keeper. It updates the keeper in place instead. +* (linting) [#12141](https://github.com/cosmos/cosmos-sdk/pull/12141) Fix usability related linting for database. This means removing the infix Prefix from `prefix.NewPrefixWriter` and such so that it is `prefix.NewWriter` and making `db.DBConnection` and such into `db.Connection` +* (x/distribution) [#12434](https://github.com/cosmos/cosmos-sdk/pull/12434) `x/distribution` module `SetParams` keeper method definition is now updated to return `error`. +* (x/staking) [#12409](https://github.com/cosmos/cosmos-sdk/pull/12409) `x/staking` module `SetParams` keeper method definition is now updated to return `error`. +* (x/crisis) [#12445](https://github.com/cosmos/cosmos-sdk/pull/12445) `x/crisis` module `SetConstantFee` keeper method definition is now updated to return `error`. +* (x/gov) [#12631](https://github.com/cosmos/cosmos-sdk/pull/12631) `x/gov` module refactored to use `Params` as single struct instead of `DepositParams`, `TallyParams` & `VotingParams`. +* (x/gov) [#12631](https://github.com/cosmos/cosmos-sdk/pull/12631) Migrate `x/gov` to self-managed parameters and deprecate it's usage of `x/params`. +* (x/bank) [#12630](https://github.com/cosmos/cosmos-sdk/pull/12630) `x/bank` module `SetParams` keeper method definition is now updated to return `error`. +* (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) Move the SendEnabled information out of the Params and into the state store directly. + The information can now be accessed using the BankKeeper. + Setting can be done using MsgSetSendEnabled as a governance proposal. + A SendEnabled query has been added to both GRPC and CLI. +* (appModule) Remove `Route`, `QuerierRoute` and `LegacyQuerierHandler` from AppModule Interface. +* (x/modules) Remove all LegacyQueries and related code from modules +* (store) [#11825](https://github.com/cosmos/cosmos-sdk/pull/11825) Make extension snapshotter interface safer to use, renamed the util function `WriteExtensionItem` to `WriteExtensionPayload`. +* (x/genutil)[#12956](https://github.com/cosmos/cosmos-sdk/pull/12956) `genutil.AppModuleBasic` has a new attribute: genesis transaction validation function. The existing validation logic is implemented in `genutiltypes.DefaultMessageValidator`. Use `genutil.NewAppModuleBasic` to create a new genutil Module Basic. +* (codec) [#12964](https://github.com/cosmos/cosmos-sdk/pull/12964) `ProtoCodec.MarshalInterface` now returns an error when serializing unregistered types and a subsequent `ProtoCodec.UnmarshalInterface` would fail. +* (x/staking) [#12973](https://github.com/cosmos/cosmos-sdk/pull/12973) Removed `stakingkeeper.RandomValidator`. Use `testutil.RandSliceElem(r, sk.GetAllValidators(ctx))` instead. +* (x/gov) [#13160](https://github.com/cosmos/cosmos-sdk/pull/13160) Remove custom marshaling of proposl and voteoption. +* (types) [#13430](https://github.com/cosmos/cosmos-sdk/pull/13430) Remove unused code `ResponseCheckTx` and `ResponseDeliverTx` +* (store) [#13529](https://github.com/cosmos/cosmos-sdk/pull/13529) Add method `LatestVersion` to `MultiStore` interface, add method `SetQueryMultiStore` to baesapp to support alternative `MultiStore` implementation for query service. +* (pruning) [#13609](https://github.com/cosmos/cosmos-sdk/pull/13609) Move pruning package to be under store package +* [#13794](https://github.com/cosmos/cosmos-sdk/pull/13794) Most methods on `types/module.AppModule` have been moved to +extension interfaces. `module.Manager.Modules` is now of type `map[string]interface{}` to support in parallel the new +`cosmossdk.io/core/appmodule.AppModule` API. +* (signing) [#13701](https://github.com/cosmos/cosmos-sdk/pull/) Add `context.Context` as an argument `x/auth/signing.VerifySignature`. +* (x/group) [#13876](https://github.com/cosmos/cosmos-sdk/pull/13876) Add `GetMinExecutionPeriod` method on DecisionPolicy interface. +* (x/auth)[#13780](https://github.com/cosmos/cosmos-sdk/pull/13780) Querying with `id` (type of int64) in `AccountAddressByID` grpc query now throws error, use account-id(type of uint64) instead. +* (snapshots) [14048](https://github.com/cosmos/cosmos-sdk/pull/14048) Move the Snapshot package to the store package. This is done in an effort group all storage related logic under one package. +* (baseapp) [#14050](https://github.com/cosmos/cosmos-sdk/pull/14050) refactor `ABCIListener` interface to accept go contexts + +### CLI Breaking Changes + +* (x/genutil) [#13535](https://github.com/cosmos/cosmos-sdk/pull/13535) Replace in `simd init`, the `--staking-bond-denom` flag with `--default-denom` which is used for all default denomination in the genesis, instead of only staking. +* (tx) [#12659](https://github.com/cosmos/cosmos-sdk/pull/12659) Remove broadcast mode `block`. +* (genesis) [#14149](https://github.com/cosmos/cosmos-sdk/pull/14149) Add `simd genesis` command, which contains all genesis-related sub-commands. + +### Bug Fixes + +* (x/upgrade) [#13936](https://github.com/cosmos/cosmos-sdk/pull/13936) Make downgrade verification work again +* (x/group) [#13742](https://github.com/cosmos/cosmos-sdk/pull/13742) Fix `validate-genesis` when group policy accounts exist. +* (x/auth) [#13838](https://github.com/cosmos/cosmos-sdk/pull/13838) Fix calling `String()` when pubkey is set on a `BaseAccount`. +* (rosetta) [#13583](https://github.com/cosmos/cosmos-sdk/pull/13583) Misc fixes for cosmos-rosetta. +* (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) Fix evidence query API to decode the hash properly. +* (bank) [#13691](https://github.com/cosmos/cosmos-sdk/issues/13691) Fix unhandled error for vesting account transfers, when total vesting amount exceeds total balance. +* [#13553](https://github.com/cosmos/cosmos-sdk/pull/13553) Ensure all parameter validation for decimal types handles nil decimal values. +* [#13145](https://github.com/cosmos/cosmos-sdk/pull/13145) Fix panic when calling `String()` to a Record struct type. +* [#13116](https://github.com/cosmos/cosmos-sdk/pull/13116) Fix a dead-lock in the `Group-TotalWeight` `x/group` invariant. +* (genutil) [#12140](https://github.com/cosmos/cosmos-sdk/pull/12140) Fix staking's genesis JSON migrate in the `simd migrate v0.46` CLI command. +* (types) [#12154](https://github.com/cosmos/cosmos-sdk/pull/12154) Add `baseAccountGetter` to avoid invalid account error when create vesting account. +* (x/authz) [#12184](https://github.com/cosmos/cosmos-sdk/pull/12184) Fix MsgExec not verifying the validity of nested messages. +* (x/staking) [#12303](https://github.com/cosmos/cosmos-sdk/pull/12303) Use bytes instead of string comparison in delete validator queue +* (store/rootmulti) [#12487](https://github.com/cosmos/cosmos-sdk/pull/12487) Fix non-deterministic map iteration. +* (sdk/dec_coins) [#12903](https://github.com/cosmos/cosmos-sdk/pull/12903) Fix nil `DecCoin` creation when converting `Coins` to `DecCoins` +* (store) [#12945](https://github.com/cosmos/cosmos-sdk/pull/12945) Fix nil end semantics in store/cachekv/iterator when iterating a dirty cache. +* (x/gov) [#13051](https://github.com/cosmos/cosmos-sdk/pull/13051) In SubmitPropsal, when a legacy msg fails it's handler call, wrap the error as ErrInvalidProposalContent (instead of ErrNoProposalHandlerExists). +* (x/gov) [#13045](https://github.com/cosmos/cosmos-sdk/pull/13045) Fix gov migrations for v3(0.46). +* (snapshot) [#13400](https://github.com/cosmos/cosmos-sdk/pull/13400) Fix snapshot checksum issue in golang 1.19. +* (server) [#13778](https://github.com/cosmos/cosmos-sdk/pull/13778) Set Cosmos SDK default endpoints to localhost to avoid unknown exposure of endpoints. +* (x/auth) [#13877](https://github.com/cosmos/cosmos-sdk/pull/13877) Handle missing account numbers during `InitGenesis`. + +### Deprecated + +* (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) The `evidence_hash` field of `QueryEvidenceRequest` has been deprecated and now contains a new field `hash` with type `string`. +* (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) The Params.SendEnabled field is deprecated and unusable. + The information can now be accessed using the BankKeeper. + Setting can be done using MsgSetSendEnabled as a governance proposal. + A SendEnabled query has been added to both GRPC and CLI. + +>>>>>>> f1ee974ec8 (refactor: make cachekv store thread-safe again (#14378)) ## [v0.46.7](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.7) - 2022-12-13 ### Features diff --git a/go.mod b/go.mod index e3e2e9dda99bb..a27daf85552f5 100644 --- a/go.mod +++ b/go.mod @@ -53,7 +53,7 @@ require ( github.com/tendermint/go-amino v0.16.0 github.com/tendermint/tendermint v0.34.24 github.com/tendermint/tm-db v0.6.7 - github.com/tidwall/btree v1.5.2 + github.com/tidwall/btree v1.6.0 golang.org/x/crypto v0.2.0 golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e google.golang.org/genproto v0.0.0-20221014213838-99cd37c6964a diff --git a/go.sum b/go.sum index c71a35a6b8c57..0be1a171cab87 100644 --- a/go.sum +++ b/go.sum @@ -952,8 +952,8 @@ github.com/tendermint/tendermint v0.34.24 h1:879MKKJWYYPJEMMKME+DWUTY4V9f/FBpnZD github.com/tendermint/tendermint v0.34.24/go.mod h1:rXVrl4OYzmIa1I91av3iLv2HS0fGSiucyW9J4aMTpKI= github.com/tendermint/tm-db v0.6.7 h1:fE00Cbl0jayAoqlExN6oyQJ7fR/ZtoVOmvPJ//+shu8= github.com/tendermint/tm-db v0.6.7/go.mod h1:byQDzFkZV1syXr/ReXS808NxA2xvyuuVgXOJ/088L6I= -github.com/tidwall/btree v1.5.2 h1:5eA83Gfki799V3d3bJo9sWk+yL2LRoTEah3O/SA6/8w= -github.com/tidwall/btree v1.5.2/go.mod h1:twD9XRA5jj9VUQGELzDO4HPQTNJsoWWfYEL+EUQ2cKY= +github.com/tidwall/btree v1.6.0 h1:LDZfKfQIBHGHWSwckhXI0RPSXzlo+KYdjK7FWSqOzzg= +github.com/tidwall/btree v1.6.0/go.mod h1:twD9XRA5jj9VUQGELzDO4HPQTNJsoWWfYEL+EUQ2cKY= github.com/tidwall/gjson v1.12.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/gjson v1.14.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= diff --git a/store/cachekv/internal/btree.go b/store/cachekv/internal/btree.go index 142f754bbd38e..d0340022f2c56 100644 --- a/store/cachekv/internal/btree.go +++ b/store/cachekv/internal/btree.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" + "github.com/cosmos/cosmos-sdk/store/types" "github.com/tidwall/btree" ) @@ -21,23 +22,24 @@ var errKeyEmpty = errors.New("key cannot be empty") // // We choose tidwall/btree over google/btree here because it provides API to implement step iterator directly. type BTree struct { - tree btree.BTreeG[item] + tree *btree.BTreeG[item] } // NewBTree creates a wrapper around `btree.BTreeG`. -func NewBTree() *BTree { - return &BTree{tree: *btree.NewBTreeGOptions(byKeys, btree.Options{ - Degree: bTreeDegree, - // Contract: cachekv store must not be called concurrently - NoLocks: true, - })} +func NewBTree() BTree { + return BTree{ + tree: btree.NewBTreeGOptions(byKeys, btree.Options{ + Degree: bTreeDegree, + NoLocks: false, + }), + } } -func (bt *BTree) Set(key, value []byte) { +func (bt BTree) Set(key, value []byte) { bt.tree.Set(newItem(key, value)) } -func (bt *BTree) Get(key []byte) []byte { +func (bt BTree) Get(key []byte) []byte { i, found := bt.tree.Get(newItem(key, nil)) if !found { return nil @@ -45,22 +47,30 @@ func (bt *BTree) Get(key []byte) []byte { return i.value } -func (bt *BTree) Delete(key []byte) { +func (bt BTree) Delete(key []byte) { bt.tree.Delete(newItem(key, nil)) } -func (bt *BTree) Iterator(start, end []byte) (*memIterator, error) { +func (bt BTree) Iterator(start, end []byte) (types.Iterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { return nil, errKeyEmpty } - return NewMemIterator(start, end, bt, make(map[string]struct{}), true), nil + return newMemIterator(start, end, bt, true), nil } -func (bt *BTree) ReverseIterator(start, end []byte) (*memIterator, error) { +func (bt BTree) ReverseIterator(start, end []byte) (types.Iterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { return nil, errKeyEmpty } - return NewMemIterator(start, end, bt, make(map[string]struct{}), false), nil + return newMemIterator(start, end, bt, false), nil +} + +// Copy the tree. This is a copy-on-write operation and is very fast because +// it only performs a shadowed copy. +func (bt BTree) Copy() BTree { + return BTree{ + tree: bt.tree.Copy(), + } } // item is a btree item with byte slices as keys and values diff --git a/store/cachekv/internal/btree_test.go b/store/cachekv/internal/btree_test.go index f85a8bbaf1095..b6aa22db8e0fa 100644 --- a/store/cachekv/internal/btree_test.go +++ b/store/cachekv/internal/btree_test.go @@ -3,6 +3,7 @@ package internal import ( "testing" + "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" ) @@ -181,7 +182,7 @@ func TestDBIterator(t *testing.T) { verifyIterator(t, ritr, nil, "reverse iterator with empty db") } -func verifyIterator(t *testing.T, itr *memIterator, expected []int64, msg string) { +func verifyIterator(t *testing.T, itr types.Iterator, expected []int64, msg string) { i := 0 for itr.Valid() { key := itr.Key() diff --git a/store/cachekv/internal/memiterator.go b/store/cachekv/internal/memiterator.go index 2bceb8bc77df0..b2fa93d3f71c9 100644 --- a/store/cachekv/internal/memiterator.go +++ b/store/cachekv/internal/memiterator.go @@ -11,20 +11,18 @@ import ( var _ types.Iterator = (*memIterator)(nil) // memIterator iterates over iterKVCache items. -// if key is nil, means it was deleted. +// if value is nil, means it was deleted. // Implements Iterator. type memIterator struct { - iter btree.GenericIter[item] + iter btree.IterG[item] start []byte end []byte ascending bool - lastKey []byte - deleted map[string]struct{} valid bool } -func NewMemIterator(start, end []byte, items *BTree, deleted map[string]struct{}, ascending bool) *memIterator { +func newMemIterator(start, end []byte, items BTree, ascending bool) *memIterator { iter := items.tree.Iter() var valid bool if ascending { @@ -52,8 +50,6 @@ func NewMemIterator(start, end []byte, items *BTree, deleted map[string]struct{} start: start, end: end, ascending: ascending, - lastKey: nil, - deleted: deleted, valid: valid, } @@ -113,21 +109,7 @@ func (mi *memIterator) Key() []byte { } func (mi *memIterator) Value() []byte { - item := mi.iter.Item() - key := item.key - // We need to handle the case where deleted is modified and includes our current key - // We handle this by maintaining a lastKey object in the iterator. - // If the current key is the same as the last key (and last key is not nil / the start) - // then we are calling value on the same thing as last time. - // Therefore we don't check the mi.deleted to see if this key is included in there. - if _, ok := mi.deleted[string(key)]; ok { - if mi.lastKey == nil || !bytes.Equal(key, mi.lastKey) { - // not re-calling on old last key - return nil - } - } - mi.lastKey = key - return item.value + return mi.iter.Item().value } func (mi *memIterator) assertValid() { diff --git a/store/cachekv/store.go b/store/cachekv/store.go index ef14ad2d5d403..bfbe1550009f3 100644 --- a/store/cachekv/store.go +++ b/store/cachekv/store.go @@ -24,14 +24,11 @@ type cValue struct { } // Store wraps an in-memory cache around an underlying types.KVStore. -// If a cached value is nil but deleted is defined for the corresponding key, -// it means the parent doesn't have the key. (No need to delete upon Write()) type Store struct { mtx sync.Mutex cache map[string]*cValue - deleted map[string]struct{} unsortedCache map[string]struct{} - sortedCache *internal.BTree // always ascending sorted + sortedCache internal.BTree // always ascending sorted parent types.KVStore } @@ -41,7 +38,6 @@ var _ types.CacheKVStore = (*Store)(nil) func NewStore(parent types.KVStore) *Store { return &Store{ cache: make(map[string]*cValue), - deleted: make(map[string]struct{}), unsortedCache: make(map[string]struct{}), sortedCache: internal.NewBTree(), parent: parent, @@ -63,7 +59,7 @@ func (store *Store) Get(key []byte) (value []byte) { cacheValue, ok := store.cache[conv.UnsafeBytesToStr(key)] if !ok { value = store.parent.Get(key) - store.setCacheValue(key, value, false, false) + store.setCacheValue(key, value, false) } else { value = cacheValue.value } @@ -79,7 +75,7 @@ func (store *Store) Set(key []byte, value []byte) { types.AssertValidKey(key) types.AssertValidValue(value) - store.setCacheValue(key, value, false, true) + store.setCacheValue(key, value, true) } // Has implements types.KVStore. @@ -94,7 +90,7 @@ func (store *Store) Delete(key []byte) { defer store.mtx.Unlock() types.AssertValidKey(key) - store.setCacheValue(key, nil, true, true) + store.setCacheValue(key, nil, true) } // Implements Cachetypes.KVStore. @@ -102,7 +98,7 @@ func (store *Store) Write() { store.mtx.Lock() defer store.mtx.Unlock() - if len(store.cache) == 0 && len(store.deleted) == 0 && len(store.unsortedCache) == 0 { + if len(store.cache) == 0 && len(store.unsortedCache) == 0 { store.sortedCache = internal.NewBTree() return } @@ -122,19 +118,16 @@ func (store *Store) Write() { // TODO: Consider allowing usage of Batch, which would allow the write to // at least happen atomically. for _, key := range keys { - if store.isDeleted(key) { - // We use []byte(key) instead of conv.UnsafeStrToBytes because we cannot - // be sure if the underlying store might do a save with the byteslice or - // not. Once we get confirmation that .Delete is guaranteed not to - // save the byteslice, then we can assume only a read-only copy is sufficient. - store.parent.Delete([]byte(key)) - continue - } - + // We use []byte(key) instead of conv.UnsafeStrToBytes because we cannot + // be sure if the underlying store might do a save with the byteslice or + // not. Once we get confirmation that .Delete is guaranteed not to + // save the byteslice, then we can assume only a read-only copy is sufficient. cacheValue := store.cache[key] if cacheValue.value != nil { - // It already exists in the parent, hence delete it. + // It already exists in the parent, hence update it. store.parent.Set([]byte(key), cacheValue.value) + } else { + store.parent.Delete([]byte(key)) } } @@ -144,9 +137,6 @@ func (store *Store) Write() { for key := range store.cache { delete(store.cache, key) } - for key := range store.deleted { - delete(store.deleted, key) - } for key := range store.unsortedCache { delete(store.unsortedCache, key) } @@ -180,16 +170,24 @@ func (store *Store) iterator(start, end []byte, ascending bool) types.Iterator { store.mtx.Lock() defer store.mtx.Unlock() - var parent, cache types.Iterator + store.dirtyItems(start, end) + isoSortedCache := store.sortedCache.Copy() + + var ( + err error + parent, cache types.Iterator + ) if ascending { parent = store.parent.Iterator(start, end) + cache, err = isoSortedCache.Iterator(start, end) } else { parent = store.parent.ReverseIterator(start, end) + cache, err = isoSortedCache.ReverseIterator(start, end) + } + if err != nil { + panic(err) } - - store.dirtyItems(start, end) - cache = internal.NewMemIterator(start, end, store.sortedCache, store.deleted, ascending) return internal.NewCacheMergeIterator(parent, cache, ascending) } @@ -364,12 +362,7 @@ func (store *Store) clearUnsortedCacheSubset(unsorted []*kv.Pair, sortState sort } for _, item := range unsorted { - if item.Value == nil { - // deleted element, tracked by store.deleted - // setting arbitrary value - store.sortedCache.Set(item.Key, []byte{}) - continue - } + // sortedCache is able to store `nil` value to represent deleted items. store.sortedCache.Set(item.Key, item.Value) } } @@ -378,23 +371,14 @@ func (store *Store) clearUnsortedCacheSubset(unsorted []*kv.Pair, sortState sort // etc // Only entrypoint to mutate store.cache. -func (store *Store) setCacheValue(key, value []byte, deleted bool, dirty bool) { +// A `nil` value means a deletion. +func (store *Store) setCacheValue(key, value []byte, dirty bool) { keyStr := conv.UnsafeBytesToStr(key) store.cache[keyStr] = &cValue{ value: value, dirty: dirty, } - if deleted { - store.deleted[keyStr] = struct{}{} - } else { - delete(store.deleted, keyStr) - } if dirty { store.unsortedCache[keyStr] = struct{}{} } } - -func (store *Store) isDeleted(key string) bool { - _, ok := store.deleted[key] - return ok -}