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

Feat/nv21 migrations optimized tree diff algorithm #11149

Merged
merged 6 commits into from
Aug 14, 2023

Conversation

snissn
Copy link
Contributor

@snissn snissn commented Aug 8, 2023

Related Issues

#11071

Proposed Changes

  • Imported v12 of the filecoin state types in migrations.go. This allows the system to interpret and operate on the latest structure of filecoin state information, ensuring compatibility with the latest version of the Filecoin network.

  • Updated the 'getMigrationFuncsForNetwork' function to include a case for network.Version21, enabling the use of the UpgradeActorsV12 and PreUpgradeActorsV12 functions for network migration. These changes ensure the system correctly handles migrations when the network upgrades to version 21.

  • Include latest version of go-state-types module which includes optimizations for AMT diffing, allowing this migration to happen swiftly

  • Added a new 'pprof' flag to the command-line arguments in main.go. This flag allows the user to specify a file name for writing CPU profile information, enabling performance analysis.

  • includes skeleton nv21 migration kit which need to be filled out by @ZenGround0 and @arajasek

Additional Info

This has been benchmarked and the optimization for AMT diffing has been verified to be correct via correct stateroot cids.

REMINDER: If you are running this, you likely want to ALSO run the continuity testing tool!
manifest cid: bafy2bzacedowjygm2fy5yryyyjcww6znqngoz4rildgouuj6uk35z7a2u5gmc
migration height  3104159
old cid  bafy2bzacebap5x7ow7xwubdnqfodveuksbp4k63oe7f6ukl752cm3vtp5m7fo
new cid  bafy2bzaced3m4tbxhq5fcybisppaiafbnguwecjxhkn46jds7y75yimrfc2vq
completed round actual (without cache), took  11m49.470263157s
manifest cid: bafy2bzacedowjygm2fy5yryyyjcww6znqngoz4rildgouuj6uk35z7a2u5gmc
completed premigration, took  10m20.103971889s
manifest cid: bafy2bzacedowjygm2fy5yryyyjcww6znqngoz4rildgouuj6uk35z7a2u5gmc
completed round actual (with cache), took  15.86165313s
Ec2 Instance Type - i4i.4xlarge
lotus branch - feat/nv21-migrations-optimized-tree
worker count - num-cpus

below without optimizations in go-state-type version:

REMINDER: If you are running this, you likely want to ALSO run the continuity testing tool!
manifest cid: bafy2bzacedowjygm2fy5yryyyjcww6znqngoz4rildgouuj6uk35z7a2u5gmc
migration height  3104159
old cid  bafy2bzacebap5x7ow7xwubdnqfodveuksbp4k63oe7f6ukl752cm3vtp5m7fo
new cid  bafy2bzaced3m4tbxhq5fcybisppaiafbnguwecjxhkn46jds7y75yimrfc2vq
completed round actual (without cache), took  16m12.368587429s
manifest cid: bafy2bzacedowjygm2fy5yryyyjcww6znqngoz4rildgouuj6uk35z7a2u5gmc
completed premigration, took  15m33.544663821s
manifest cid: bafy2bzacedowjygm2fy5yryyyjcww6znqngoz4rildgouuj6uk35z7a2u5gmc
completed round actual (with cache), took  1m43.923645056s
Ec2 Instance Type - i4i.4xlarge
lotus branch - feat/nv21-migrations-perf
worker count - num-cpus

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

This commit includes the following updates:

- Imported v12 of the filecoin state types in migrations.go. This allows the system to interpret and operate on the latest structure of filecoin state information, ensuring compatibility with the latest version of the Filecoin network.

- Updated the 'getMigrationFuncsForNetwork' function to include a case for network.Version21, enabling the use of the UpgradeActorsV12 and PreUpgradeActorsV12 functions for network migration. These changes ensure the system correctly handles migrations when the network upgrades to version 21.

- Implemented a new 'checkNv21Invariants' function to check the invariants for version 21 of the Filecoin network. This function:

  1. Loads the new state root from the actor store.
  2. Retrieves the actor code IDs for the current state version (v12).
  3. Loads the actor tree from the state root and checks state invariants using the actor code IDs.
  4. Logs any error messages generated during the invariant check.
  5. Finally, prints a message with the time taken to complete the checks.

By checking these invariants, we can ensure the system's state is consistent and valid after migrating to network version 21.
This commit includes the following updates:

- Added a new 'pprof' flag to the command-line arguments in main.go. This flag allows the user to specify a file name for writing CPU profile information, enabling performance analysis.

- Implemented functionality to start CPU profiling in the 'Before' function. If a file name is provided via the 'pprof' flag, the function creates the specified file and begins CPU profiling.

- Added an 'After' function to stop CPU profiling once the program execution is finished.

These changes provide a way to analyze the CPU usage of the 'lotus-shed' application, which can be valuable for performance tuning and optimization.
@snissn snissn marked this pull request as ready for review August 8, 2023 22:24
@snissn snissn requested a review from a team as a code owner August 8, 2023 22:24
@snissn snissn marked this pull request as draft August 8, 2023 22:24
@jennijuju jennijuju linked an issue Aug 9, 2023 that may be closed by this pull request
@snissn snissn marked this pull request as ready for review August 10, 2023 00:22
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

LGTM -- there's a couple more checks I'd like to add, happy to push a commit to this branch or have that be a separate PR.

Thank you!

@snissn
Copy link
Contributor Author

snissn commented Aug 14, 2023

Hey! It'd be a lot simpler for me if we could land this PR then create your check as additional work in a new PR

@arajasek arajasek merged commit fa0e512 into feat/nv21-skeleton Aug 14, 2023
@arajasek arajasek deleted the feat/nv21-migrations-optimized-tree branch August 14, 2023 21:05
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.

Integrate, benchmark, and optimize the nv21 SectorInfo migration
2 participants