Skip to content

feat: make LazyLoadVersion validate InitialVersion the same as LoadVersion#638

Merged
tac0turtle merged 6 commits intocosmos:masterfrom
yihuang:lazy-by-default
Jan 18, 2023
Merged

feat: make LazyLoadVersion validate InitialVersion the same as LoadVersion#638
tac0turtle merged 6 commits intocosmos:masterfrom
yihuang:lazy-by-default

Conversation

@yihuang
Copy link

@yihuang yihuang commented Dec 1, 2022 •
edited
Loading

Improve startup time of archive nodes.

  • check opts.InitialVersion in LazyLoadVersion
  • add API LazyLoadVersionForOverwriting

Closes: #637

Need to enable the lazy mode at cosmos-sdk: cosmos/cosmos-sdk#14189
Unit tests passed, running alright on our production nodes, not sure of other side effects.

@yihuang yihuang changed the title support lazy mode in LoadVersion feat: support lazy mode in LoadVersion Dec 1, 2022
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 1, 2022
View reviewed changes
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Dec 1, 2022

This pull request introduces 2 alerts when merging 8c7a745 into 12381ab - view on LGTM.com

new alerts:

  • 2 for Unreachable statement

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed 🚀. For more information, please check out our post on the GitHub blog.

@yihuang yihuang marked this pull request as ready for review December 1, 2022 04:00
@yihuang yihuang requested a review from a team December 1, 2022 04:00
@yihuang yihuang mentioned this pull request Dec 1, 2022
Merged
@pysel pysel mentioned this pull request Dec 3, 2022
Merged
3 tasks
@yihuang yihuang mentioned this pull request Dec 6, 2022
Closed
@yihuang yihuang changed the title feat: support lazy mode in LoadVersion feat: make LazyLoadVersion do the same thing as LoadVersion Dec 7, 2022
@yihuang yihuang mentioned this pull request Dec 7, 2022
Merged
19 tasks
yihuang
yihuang commented Jan 16, 2023
View reviewed changes
@yihuang
Copy link
Author

yihuang commented Jan 16, 2023

@alexanderbez @tac0turtle Hi, may I know what's your optional on this change?
it's a simple change, the end purpose is to add iavl-lazy-loading option to sdk(cosmos/cosmos-sdk#14189), which provides a huge UX improvement on node start up.

tac0turtle
tac0turtle reviewed Jan 16, 2023
View reviewed changes
Copy link
Contributor

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

could we get a test case on this feature, seems like some behaviour changed but the tests didnt

@yihuang
Copy link
Author

yihuang commented Jan 17, 2023

could we get a test case on this feature, seems like some behaviour changed but the tests didnt

unit test added, and I checked it fails on master branch for this error: "An error is expected but got nil."

@yihuang yihuang changed the title feat: make LazyLoadVersion do the same thing as LoadVersion feat: make LazyLoadVersion validate InitialVersion the same as LoadVersion Jan 17, 2023
cool-develope
cool-develope reviewed Jan 17, 2023
View reviewed changes
@tac0turtle
Copy link
Contributor

tac0turtle commented Jan 18, 2023

@yihuang backport?

@tac0turtle tac0turtle merged commit d8e1e38 into cosmos:master Jan 18, 2023
@yihuang
Copy link
Author

yihuang commented Jan 18, 2023 •
edited
Loading

@yihuang backport?

yes, please.

@yihuang yihuang deleted the lazy-by-default branch January 18, 2023 14:58
@yihuang
Copy link
Author

yihuang commented Jan 29, 2023

it seems we forgot to backport this one? @tac0turtle

@tac0turtle
Copy link
Contributor

tac0turtle commented Jan 29, 2023

no problem, didnt cut final release

@tac0turtle
Copy link
Contributor

tac0turtle commented Jan 29, 2023

@Mergifyio backport release/v0.19.x

@tac0turtle
Copy link
Contributor

tac0turtle commented Jan 29, 2023

ill fix the bot, not sure what's up

@mergify mergify bot mentioned this pull request Jan 29, 2023
Merged
@mergify
Copy link
Contributor

mergify bot commented Jan 29, 2023

backport release/v0.19.x

✅ Backports have been created

Details

@tac0turtle
Copy link
Contributor

tac0turtle commented Jan 30, 2023

@Mergifyio backport release/v0.20.x

@mergify mergify bot mentioned this pull request Jan 30, 2023
Merged
@mergify
Copy link
Contributor

mergify bot commented Jan 30, 2023

backport release/v0.20.x

✅ Backports have been created

Details

tac0turtle added a commit that referenced this pull request Jan 30, 2023
@mergify @tac0turtle
feat: make LazyLoadVersion validate InitialVersion the same as `L…
7afdfcd 
…oadVersion` (backport #638) (#669)

Co-authored-by: yihuang <huang@crypto.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
tac0turtle added a commit that referenced this pull request Feb 1, 2023
@mergify @tac0turtle
feat: make LazyLoadVersion validate InitialVersion the same as `L…
ddbee02 
…oadVersion` (backport #638) (#666)

Co-authored-by: yihuang <huang@crypto.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
ankurdotb pushed a commit to cheqd/iavl that referenced this pull request Feb 28, 2023
@mergify @tac0turtle @ankurdotb
feat: make LazyLoadVersion validate InitialVersion the same as `L…
fa59db2 
…oadVersion` (backport cosmos#638) (cosmos#666)

Co-authored-by: yihuang <huang@crypto.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
@coderabbitai coderabbitai bot mentioned this pull request May 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@tac0turtle tac0turtle tac0turtle approved these changes

@cool-develope cool-develope cool-develope approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alexanderbez alexanderbez

@facundomedica facundomedica

@cool-develope cool-develope

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

iavl start up time is slow

5 participants

@yihuang @tac0turtle @cool-develope @alexanderbez @facundomedica