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.

Sign up for GitHub

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

Jump to bottom

Problem: no compact storage for archive node #791

Merged
merged 6 commits into from
Jan 31, 2023
Merged

Problem: no compact storage for archive node #791

merged 6 commits into from
Jan 31, 2023

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Dec 14, 2022 •
edited
Loading

Closes: #704

Solution:

  • patch iavl to support extracting state changes
  • implement versiondb using rocksdb user-defined timestamp feature
  • now librocksdb is always required to build cronosd
    versiondb is only enabled if binary is built with rocksdb backend.
  • implement cli commands to manage the migration process:
$ ./build/cronosd changeset
dump and manage change sets files and ingest into versiondb

Usage:
  cronosd changeset [command]

Available Commands:
  convert-to-sst Convert change set files to versiondb/rocksdb sst files, which can be ingested into versiondb later
  dump           Extract changesets from iavl versions, and save to plain file format
  ingest-sst     Ingest sst files into versiondb
  print          Pretty-print the content of change set file
  to-versiondb   Feed change set files into versiondb
  verify         Replay the input change set files in order to rebuild iavl tree in memory and output root hash, user can compare the root hash against the on chain hash

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

PR Checklist:

  • Have you read the CONTRIBUTING.md?
  • Does your PR follow the C4 patch requirements?
  • Have you rebased your work on top of the latest master?
  • Have you checked your code compiles? (make)
  • Have you included tests for any non-trivial functionality?
  • Have you checked your code passes the unit tests? (make test)
  • Have you checked your code formatting is correct? (go fmt)
  • Have you checked your basic code style is fine? (golangci-lint run)
  • If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
  • If your changes affect the client infrastructure, have you run the integration test?
  • If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
  • If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
  • If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

@yihuang yihuang force-pushed the state-changes-cmd branch 7 times, most recently from 2b393e7 to 5bca411 Compare December 16, 2022 07:31
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 23, 2022
View reviewed changes
cmd/cronosd/cmd/changeset.go Fixed Show fixed Hide fixed
cmd/cronosd/cmd/changeset.go Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 23, 2022
View reviewed changes
cmd/cronosd/cmd/changeset.go Fixed Show fixed Hide fixed
cmd/cronosd/cmd/changeset.go Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 24, 2022
View reviewed changes
cmd/cronosd/cmd/changeset.go Fixed Show fixed Hide fixed
cmd/cronosd/cmd/changeset.go Fixed Show fixed Hide fixed
cmd/cronosd/cmd/changeset.go Fixed Show fixed Hide fixed
cmd/cronosd/cmd/changeset.go Fixed Show fixed Hide fixed
@yihuang yihuang mentioned this pull request Dec 27, 2022
Closed
13 tasks
tomtau
tomtau reviewed Jan 3, 2023
View reviewed changes
Copy link
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

will there be any fallback for other db backends?

cmd/cronosd/cmd/changeset.go Outdated Show resolved Hide resolved
@yihuang
Copy link
Collaborator Author

yihuang commented Jan 3, 2023 •
edited
Loading

will there be any fallback for other db backends?

that would be the original proposal, if there are any interests for that.
user can still specify different db backends for tm and iavl db though, just the versiondb will be always rocksdb for this implementation.

@yihuang yihuang force-pushed the state-changes-cmd branch 2 times, most recently from 64841d4 to a179364 Compare January 3, 2023 08:58
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 3, 2023
View reviewed changes
versiondb/multistore.go Fixed Show fixed Hide fixed
versiondb/multistore.go Fixed Show fixed Hide fixed
versiondb/multistore.go Fixed Show fixed Hide fixed
app/app.go Fixed Show fixed Hide fixed
versiondb/client/print_plain_file.go Fixed Show fixed Hide fixed
versiondb/client/dump_file.go Fixed Show fixed Hide fixed
yihuang
yihuang commented Jan 3, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
yihuang
yihuang commented Jan 3, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
yihuang
yihuang commented Jan 3, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
yihuang
yihuang commented Jan 3, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 3, 2023
View reviewed changes
versiondb/client/utils.go Fixed Show fixed Hide fixed
@codecov
Copy link

codecov bot commented Jan 4, 2023 •
edited
Loading

Codecov Report

Merging #791 (6193698) into release/v1.0.x (4e4b015) will increase coverage by 6.86%.
The diff coverage is 46.75%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           release/v1.0.x     #791      +/-   ##
==================================================
+ Coverage           34.57%   41.43%   +6.86%     
==================================================
  Files                  28       52      +24     
  Lines                1481     3393    +1912     
==================================================
+ Hits                  512     1406     +894     
- Misses                916     1826     +910     
- Partials               53      161     +108
Impacted Files Coverage Δ
app/versiondb_placeholder.go 0.00% <0.00%> (ø)
versiondb/client/cmd.go 0.00% <0.00%> (ø)
versiondb/client/dump.go 0.00% <0.00%> (ø)
versiondb/client/ingest_sst.go 0.00% <0.00%> (ø)
versiondb/client/open_db.go 0.00% <0.00%> (ø)
versiondb/client/print.go 0.00% <0.00%> (ø)
versiondb/client/to_versiondb.go 0.00% <0.00%> (ø)
versiondb/client/verify.go 0.00% <0.00%> (ø)
versiondb/client/wrapreader.go 0.00% <0.00%> (ø)
versiondb/client/convert_to_sst.go 36.07% <36.07%> (ø)
... and 14 more

@yihuang yihuang force-pushed the state-changes-cmd branch 2 times, most recently from 1df5cb0 to 5106499 Compare January 30, 2023 08:51
tomtau
tomtau approved these changes Jan 31, 2023
View reviewed changes
gomod2nix.toml Outdated Show resolved Hide resolved
versiondb/client/changeset.go Outdated Show resolved Hide resolved
versiondb/client/convert_to_sst.go Outdated Show resolved Hide resolved
versiondb/client/wrapreader.go Outdated Show resolved Hide resolved
versiondb/memiavl/README.md Outdated Show resolved Hide resolved
versiondb/memiavl/README.md Outdated Show resolved Hide resolved
versiondb/memiavl/README.md Outdated Show resolved Hide resolved
@yihuang
Copy link
Collaborator Author

yihuang commented Jan 31, 2023

it seems can't disable the typecheck linter in CI? it don't reproduce locally, will ignore for now.
related: golangci/golangci-lint-action#624

@yihuang
Problem: no compact storage for archive node
42cb7ec 
Solution:
- implement versiondb using rocksdb user-defined timestamp
- implement migration tools
@yihuang
try to fix typecheck linter errors
642b944
@yihuang
Copy link
Collaborator Author

yihuang commented Jan 31, 2023

it seems can't disable the typecheck linter in CI? it don't reproduce locally, will ignore for now. related: golangci/golangci-lint-action#624

finally figured out, typecheck error just means it fails to compile somewhere, it's CGO dependencies not setup in our case.

@yihuang yihuang force-pushed the state-changes-cmd branch 3 times, most recently from fafcac9 to 9227c10 Compare January 31, 2023 18:16
@yihuang yihuang enabled auto-merge (squash) January 31, 2023 19:23
@yihuang yihuang merged commit 361f492 into crypto-org-chain:release/v1.0.x Jan 31, 2023
This was referenced Feb 13, 2023
Closed
Closed
@coderabbitai coderabbitai bot mentioned this pull request Apr 19, 2024
Merged
13 tasks
This was referenced Apr 26, 2024
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmsqe mmsqe mmsqe left review comments

@calvinaco calvinaco calvinaco left review comments

@tomtau tomtau tomtau approved these changes

@devashishdxt devashishdxt Awaiting requested review from devashishdxt devashishdxt is a code owner automatically assigned from crypto-org-chain/cronos-dev

@leejw51crypto leejw51crypto Awaiting requested review from leejw51crypto leejw51crypto is a code owner automatically assigned from crypto-org-chain/cronos-dev

@JayT106 JayT106 Awaiting requested review from JayT106

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yihuang @JayT106 @tomtau @mmsqe @calvinaco