-
Notifications
You must be signed in to change notification settings - Fork 159
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: support hardlink in --import-mode #4757
Conversation
b52ad25
to
5fefd29
Compare
[network] | ||
bootstrap_peers = ["/dns/forest-bootstrapper/tcp/$FOREST_P2P_PORT/p2p/$${PEER_ID}"] | ||
EOF | ||
|
||
forest --chain ${CHAIN} --encrypt-keystore false --no-gc \ | ||
--config config.toml \ | ||
--rpc-address 0.0.0.0:${FOREST_RPC_PORT} \ | ||
--import-snapshot $(ls /data/*.car.zst | tail -n 1) \ | ||
--import-mode=move |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was previously copy & delete instead of rename (which takes 13s in https://github.com/ChainSafe/forest/actions/runs/10783688527/job/29906201147#step:5:121)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now hardlink works which takes 0s
https://github.com/ChainSafe/forest/actions/runs/10791399010/job/29928980199#step:5:126
2024-09-10T11:27:58.753945Z INFO forest_filecoin::daemon::db_util: Hardlinking /data/forest_snapshot_calibnet_2024-09-10_height_1954656.forest.car.zst to /data/forest/calibnet/0.19.2/car_db/1725967678753.forest.car.zst
2024-09-10T11:27:58.754386Z INFO forest_filecoin::rpc: Ready for RPC connections
2024-09-10T11:27:58.755683Z INFO forest_filecoin::daemon::db_util: Imported snapshot in: 0s, heaviest tipset epoch: 1954656
9b882f4
to
00c6035
Compare
00c6035
to
3dcd5c8
Compare
CHANGELOG.md
Outdated
- [#4757](https://github.com/ChainSafe/forest/pull/4757) Added an option to | ||
hardlink snapshots and fallback to copying them if not applicable. This can be | ||
invoked with `--import-snapshot <path> --import-mode=auto`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. (Although user experience wise this change should be transparent to users)
std::os::unix::fs::symlink(from_path, &forest_car_db_path) | ||
.context("Error creating symlink")?; | ||
} else { | ||
bail!("Snapshot file must be a valid forest.car.zst file"); | ||
} | ||
} | ||
ImportMode::Hardlink => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add tests to different snapshot import modes in src/test
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests updated to cover auto
and hardlink
cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely! I knew I created them somewhere but couldn't find them. 🤣
Summary of changes
As follow up of #4620
Changes introduced in this pull request:
--import-mode=hardlink
to hard link snapshots instead of copying them to save time and disk space--import-mode=auto
as default to hard link snapshots and fallback to copying them.Reference issue to close (if applicable)
Closes
Other information and links
Change checklist