feat: async commit #16175
feat: async commit #16175
Conversation
Closes: cosmos#16173
x/upgrade/module.go
Outdated
| Key *store.KVStoreKey | ||
| Cdc codec.Codec | ||
| AddressCodec address.Codec | ||
| CommitMultiStore store.CommitMultiStore |
There was a problem hiding this comment.
Have no idea if it's correct, not familiar with the new depinject stuff.
There was a problem hiding this comment.
This is correct, but you need something that will provide the CommitMultiStore().
There was a problem hiding this comment.
Where should I put that?
There was a problem hiding this comment.
Given that it is instantiated after appBuilder.Build, AFAIK you actually need to use a setter and not have this in the keeper, and do something like PopulateVersionMap but for the CMS.
Then you can invoke that function in the init function. I'd say something like this in x/upgrade/module.go:
appmodule.Register(&modulev1.Module{},
appmodule.Invoke(PopulateVersionMap),
+ appmodule.Invoke(PopulateCommitMultiStore),
)func PopulateCommitMultiStore(upgradeKeeper *keeper.Keeper, bApp *baseapp.BaseApp) {
if upgradeKeeper == nil {
return
}
upgradeKeeper.SetCommitMultiStore(bApp.CommitMultiStore())
}Note, that I have not tested the above.
store/iavl/store.go
Outdated
| @@ -23,6 +24,7 @@ import ( | |||
|
|
|||
| const ( | |||
| DefaultIAVLCacheSize = 500000 | |||
| CommitQueueBuffer = 10 // TODO configurable | |||
There was a problem hiding this comment.
| CommitQueueBuffer = 10 // TODO configurable | |
| CommitQueueBuffer = 10 // maximum pending commit versions. |
I guess we don't actually need to make it configurable?
| @@ -73,6 +73,10 @@ func BeginBlocker(k *keeper.Keeper, ctx sdk.Context) { | |||
| panic(fmt.Errorf("unable to write upgrade info to filesystem: %s", err.Error())) | |||
| } | |||
|
|
|||
| if err := k.WaitAsyncCommit(); err != nil { | |||
There was a problem hiding this comment.
| if err := k.WaitAsyncCommit(); err != nil { | |
| // avoid state rollback after restart | |
| if err := k.WaitAsyncCommit(); err != nil { |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for _, store := range rs.stores { | ||
| errs = append(errs, store.WaitAsyncCommit()) | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map
| go func() { | ||
| defer close(quitChan) | ||
|
|
||
| for expVersion := range commitQueue { | ||
| _, version, err := st.tree.SaveVersion() | ||
| if err != nil { | ||
| quitChan <- err | ||
| break | ||
| } | ||
|
|
||
| if version != expVersion { | ||
| quitChan <- fmt.Errorf("version sanity check failed: %d != %d", expVersion, version) | ||
| break | ||
| } | ||
| } | ||
| }() |
Check notice
Code scanning / CodeQL
Spawning a Go routine
store/rootmulti/store.go
Outdated
| for key, store := range rs.stores { | ||
| if store.GetStoreType() == types.StoreTypeIAVL { | ||
| // If the store is wrapped with an inter-block cache, we must first unwrap | ||
| // it to get the underlying IAVL store. | ||
| store = rs.GetCommitKVStore(key) | ||
| store.(types.StoreWithCommitBufferSize).SetCommitBufferSize(size) | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map
|
don't work with current iavl implementation yet, see #16173 for discussions. |
Description
Closes: #16173
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change