-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Add in-place store migrations #8485
Merged
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
10d72d7
Add 1st version of migrate
amaury1093 8036fca
Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-8…
amaury1093 a9a406e
Put migration logic into Configurator
amaury1093 9ed3987
add test to bank store migration
amaury1093 a095d3a
add test for configurator
amaury1093 d550bff
Merge branch 'master' into am-8345-migration
amaury1093 b7141f9
Error if no migration found
amaury1093 2d554ce
Remove RunMigrations from Configurator interface
amaury1093 3df3f44
Update spec
amaury1093 ba6bd44
Rename folders
amaury1093 197b7ce
Merge branch 'master' into am-8345-migration
amaury1093 424932c
copy-paste from keys.go
amaury1093 254a71f
Merge branch 'am-8345-migration' of ssh://github.com/cosmos/cosmos-sd…
amaury1093 b6c0714
Fix nil map
amaury1093 9ab7f13
rename function
amaury1093 fc076f5
Update simapp/app.go
amaury1093 ae7b7dc
Update simapp/app_test.go
amaury1093 2c26734
Adderss reviews
amaury1093 eb15047
Merge branch 'am-8345-migration' of ssh://github.com/cosmos/cosmos-sd…
amaury1093 291a9e6
Fix tests
amaury1093 41d29ed
Update testutil/context.go
amaury1093 33494dc
Update docs for ConsensusVersion
amaury1093 e3a2412
Rename to forVersion
amaury1093 6db31d4
Merge branch 'am-8345-migration' of ssh://github.com/cosmos/cosmos-sd…
amaury1093 55f547e
Fix tests
amaury1093 8f3b9d4
Check error early
amaury1093 29b85b7
Return 1 for intiial version
amaury1093 41e0339
Use MigrationKeeper
amaury1093 d922003
Fix test
amaury1093 f422b93
Revert adding marshaler to Configurator
amaury1093 8cf88fe
Godoc updates
amaury1093 5add13a
Update docs
amaury1093 2cef291
Merge branch 'master' into am-8345-migration
aaronc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package testutil | ||
|
||
import ( | ||
"github.com/tendermint/tendermint/libs/log" | ||
tmproto "github.com/tendermint/tendermint/proto/tendermint/types" | ||
dbm "github.com/tendermint/tm-db" | ||
|
||
"github.com/cosmos/cosmos-sdk/store" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
) | ||
|
||
// DefaultContext creates a sdk.Context with a fresh MemDB that can be used in tests. | ||
func DefaultContext(key sdk.StoreKey, tkey sdk.StoreKey) sdk.Context { | ||
db := dbm.NewMemDB() | ||
cms := store.NewCommitMultiStore(db) | ||
cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db) | ||
cms.MountStoreWithDB(tkey, sdk.StoreTypeTransient, db) | ||
err := cms.LoadLatestVersion() | ||
if err != nil { | ||
panic(err) | ||
} | ||
ctx := sdk.NewContext(cms, tmproto.Header{}, false, log.NewNopLogger()) | ||
|
||
return ctx | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,11 @@ | ||||||||
package module | ||||||||
|
||||||||
import "github.com/gogo/protobuf/grpc" | ||||||||
import ( | ||||||||
"github.com/gogo/protobuf/grpc" | ||||||||
|
||||||||
sdk "github.com/cosmos/cosmos-sdk/types" | ||||||||
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||||||||
) | ||||||||
|
||||||||
// Configurator provides the hooks to allow modules to configure and register | ||||||||
// their services in the RegisterServices method. It is designed to eventually | ||||||||
|
@@ -15,16 +20,34 @@ type Configurator interface { | |||||||
// QueryServer returns a grpc.Server instance which allows registering services | ||||||||
// that will be exposed as gRPC services as well as ABCI query handlers. | ||||||||
QueryServer() grpc.Server | ||||||||
|
||||||||
// RegisterMigration registers an in-place store migration for a module. The | ||||||||
// handler is a migration script to perform in-place migrations from version | ||||||||
// `forVersion` to version `forVersion+1`. | ||||||||
// | ||||||||
// EACH TIME a module's ConsensusVersion increments, a new migration MUST | ||||||||
// be registered using this function. If a migration handler is missing for | ||||||||
// a particular function, the upgrade logic (see RunMigrations function) | ||||||||
// will panic. If the ConsensusVersion bump does not introduce any store | ||||||||
// changes, then a no-op function must be registered here. | ||||||||
RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error | ||||||||
} | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
|
||||||||
type configurator struct { | ||||||||
msgServer grpc.Server | ||||||||
queryServer grpc.Server | ||||||||
|
||||||||
// migrations is a map of moduleName -> forVersion -> migration script handler | ||||||||
migrations map[string]map[uint64]MigrationHandler | ||||||||
} | ||||||||
|
||||||||
// NewConfigurator returns a new Configurator instance | ||||||||
func NewConfigurator(msgServer grpc.Server, queryServer grpc.Server) Configurator { | ||||||||
return configurator{msgServer: msgServer, queryServer: queryServer} | ||||||||
return configurator{ | ||||||||
msgServer: msgServer, | ||||||||
queryServer: queryServer, | ||||||||
migrations: map[string]map[uint64]MigrationHandler{}, | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
var _ Configurator = configurator{} | ||||||||
|
@@ -38,3 +61,51 @@ func (c configurator) MsgServer() grpc.Server { | |||||||
func (c configurator) QueryServer() grpc.Server { | ||||||||
return c.queryServer | ||||||||
} | ||||||||
|
||||||||
// RegisterMigration implements the Configurator.RegisterMigration method | ||||||||
func (c configurator) RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error { | ||||||||
if forVersion == 0 { | ||||||||
return sdkerrors.Wrap(sdkerrors.ErrInvalidVersion, "module migration versions should start at 1") | ||||||||
} | ||||||||
|
||||||||
if c.migrations[moduleName] == nil { | ||||||||
c.migrations[moduleName] = map[uint64]MigrationHandler{} | ||||||||
} | ||||||||
|
||||||||
if c.migrations[moduleName][forVersion] != nil { | ||||||||
odeke-em marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
return sdkerrors.Wrapf(sdkerrors.ErrLogic, "another migration for module %s and version %d already exists", moduleName, forVersion) | ||||||||
} | ||||||||
|
||||||||
c.migrations[moduleName][forVersion] = handler | ||||||||
|
||||||||
return nil | ||||||||
} | ||||||||
|
||||||||
// runModuleMigrations runs all in-place store migrations for one given module from a | ||||||||
// version to another version. | ||||||||
func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { | ||||||||
// No-op if toVersion is the initial version. | ||||||||
if toVersion <= 1 { | ||||||||
return nil | ||||||||
} | ||||||||
|
||||||||
moduleMigrationsMap, found := c.migrations[moduleName] | ||||||||
if !found { | ||||||||
return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migrations found for module %s", moduleName) | ||||||||
} | ||||||||
|
||||||||
// Run in-place migrations for the module sequentially until toVersion. | ||||||||
for i := fromVersion; i < toVersion; i++ { | ||||||||
migrateFn, found := moduleMigrationsMap[i] | ||||||||
if !found { | ||||||||
return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) | ||||||||
amaury1093 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
} | ||||||||
|
||||||||
err := migrateFn(ctx) | ||||||||
if err != nil { | ||||||||
return err | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
return nil | ||||||||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a functional tests. Shall we add:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a bad idea to distinguish short and verbose tests going forward...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Though the
Makefile
currently does not support-short
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should park this aside for a second, identify all the various test types (short, verbose, etc), introduce the necessary changes in the
Makefile
(I'm happy to take care of it), and take it from thereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alessio thanks, sounds good. In this case, I didn't add
testing.Short()
in this test.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this tests runs below 0.1s then fine. Otherwise let's add
TODO: check testing.Short() flag
. 100 tests of 0.1 already give 10s ;)