feat: generate .forest.car.zst files by default #3283
Conversation
lemmih
requested review from
hanabi1224 and
sudo-shashank
and removed request for
a team
| tipset: &Tipset, | ||
| lookup_depth: ChainEpochDelta, | ||
| writer: W, | ||
| compressed: bool, | ||
| writer: impl AsyncWrite + Unpin, |
There was a problem hiding this comment.
How about writer: impl AsyncBufWrite + Unpin to enforce using BufWriter here
There was a problem hiding this comment.
Do we want to enforce that?
There was a problem hiding this comment.
I see most of the export calls are passing File instead of BufWriter, should it be beneficial to use BufWriter given these exported snapshots are pretty large? if it is, should we enforce using BufWriter?
There was a problem hiding this comment.
BufWriter isn't automatically better, and it only improves performance when you have a lot of small writes. We do not have a lot of small writes.
| @@ -54,6 +54,8 @@ | |||
| lookup errors instead of silently ignoring them. | |||
| - [#2999](https://github.com/ChainSafe/forest/issues/2999): Restored `--tipset` | |||
| flag to `forest-cli snapshot export` to allow export at a specific tipset. | |||
| - [#3283](https://github.com/ChainSafe/forest/pull/3283): All generated car | |||
There was a problem hiding this comment.
It's a breaking change, right?
There was a problem hiding this comment.
In other words, should any services (e.g., upload snapshot) be updated to this logic?
There was a problem hiding this comment.
Surprisingly it is not! The files are all backward compatible. The new snapshots can still be used with old versions of Forest or even with Lotus. They just won't get any of the new cool benefits.
There was a problem hiding this comment.
Are we also good with extensions and many regular expressions to match files?
There was a problem hiding this comment.
Oh,yeah, we'll need to update some regexes.
|
It'd be nice to update tests to enforce this behaviour. |
Summary of changes
Changes introduced in this pull request:
forest-cli snapshot exportgenerates.forest.car.zstoutput,forest-cli archive exportgenerates.forest.car.zstoutput.now()) when exporting.It's more work to generate
.forest.car.zstfiles but the new encoder is much faster than the old one. All in all, exporting is now a lot faster and the generated compressed files can be loaded directly into Forest.Reference issue to close (if applicable)
Closes
Other information and links
Change checklist