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] fix state_sync #150

Merged
merged 2 commits into from
Jun 19, 2019
Merged

[R4R] fix state_sync #150

merged 2 commits into from
Jun 19, 2019

Conversation

ackratos
Copy link
Contributor

@ackratos ackratos commented Jun 19, 2019

This fixes newly added store might broken statesync.

This fix makes state sync helper no longer cache mounted stores (as they may dynamically change according to upgrade) and load needed stores according to upgrade height.

cosmos build and unit test can pass
node manually test (state sync before and after upgrade) can pass

@ackratos ackratos requested review from HaoyangLiu and yutianwu June 19, 2019 02:24
@ackratos ackratos self-assigned this Jun 19, 2019
@ackratos ackratos changed the title fix state_sync [WIP] fix state_sync Jun 19, 2019
@ackratos ackratos force-pushed the statesync_upgrade_fix branch from fc532fd to c49ad65 Compare June 19, 2019 02:30
@ackratos ackratos changed the title [WIP] fix state_sync [R4R] fix state_sync Jun 19, 2019
names := make([]string, 0, len(kvStores))
nameToKey := make(map[string]sdk.StoreKey, len(kvStores))
for key, store := range cms.GetCommitKVStores() {
for key, store := range helper.commitMS.GetCommitKVStores() {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use kvStores above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my brain was dead

ackratos added a commit to bnb-chain/node that referenced this pull request Jun 19, 2019
names := make([]string, 0, len(kvStores))
nameToKey := make(map[string]sdk.StoreKey, len(kvStores))
for key, store := range cms.GetCommitKVStores() {
for key, store := range helper.commitMS.GetCommitKVStores() {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about replacing helper.commitMS.GetCommitKVStores() with kvStores? Here you call helper.commitMS.GetCommitKVStores() twice.

@ackratos ackratos force-pushed the statesync_upgrade_fix branch from aa91c18 to c530429 Compare June 19, 2019 06:13
ackratos added a commit to bnb-chain/node that referenced this pull request Jun 19, 2019
@ackratos ackratos force-pushed the statesync_upgrade_fix branch from c530429 to 99f7fee Compare June 19, 2019 06:41
@ackratos ackratos merged commit 93ade5f into develop Jun 19, 2019
@forcodedancing forcodedancing deleted the statesync_upgrade_fix branch May 18, 2022 07:09
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
forcodedancing pushed a commit to bnb-chain/node that referenced this pull request May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants