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: populate the index on snapshot import #10556

Merged
merged 6 commits into from
Mar 28, 2023

Conversation

fridrik01
Copy link
Contributor

@fridrik01 fridrik01 commented Mar 24, 2023

Related Issues

Fixes: #10537

Proposed Changes

After importing from snapshot this PR creates and populates the sqlite message index introduced in #10452 from the 2k tipsets in the snapshot.

Additional Info

When loading from snapshot from mainnet, populating the message index takex approx 30 seconds. We should be able to lower this somewhat with bulk inserts (though doing it inside a transaction is at least an order of magnitude faster than not).

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

@fridrik01 fridrik01 force-pushed the 10537-populate-index-after-snapshot branch from 92891df to 51b3ec4 Compare March 24, 2023 11:40
@fridrik01 fridrik01 force-pushed the 10537-populate-index-after-snapshot branch from 51b3ec4 to 59640a8 Compare March 24, 2023 11:41
@fridrik01 fridrik01 changed the title Populate the index on snapshot import feat: populate the index on snapshot import Mar 24, 2023
@fridrik01 fridrik01 marked this pull request as ready for review March 24, 2023 15:52
@fridrik01 fridrik01 requested a review from a team as a code owner March 24, 2023 15:52
Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

overall lgtm, just a small comment.


dbPath := path.Join(basePath, dbName)
if _, err := os.Stat(dbPath); err == nil {
return xerrors.Errorf("msgindex already exists at %s", dbPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just delete it and recreate it?
Sounds like the right thing to do.

@fridrik01 fridrik01 force-pushed the 10537-populate-index-after-snapshot branch from e757030 to b8137f6 Compare March 24, 2023 16:45
@fridrik01
Copy link
Contributor Author

@vyzo I double checked and verified that the lotus daemon continues to populate and update the sqlite/msgindex.db created by populating the message index after loading from snapshot, so looking good.

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

a couple of small things.

chain/index/msgindex.go Show resolved Hide resolved
chain/index/msgindex.go Outdated Show resolved Hide resolved
@arajasek arajasek merged commit 687f0c0 into master Mar 28, 2023
@arajasek arajasek deleted the 10537-populate-index-after-snapshot branch March 28, 2023 15:41
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.

Populate the index on snapshot import
3 participants