feat: add blockstore that is the union of several CAR stores #3288

lemmih · 2023-07-30T15:04:58Z

Summary of changes

Changes introduced in this pull request:

add ManyCar blockstore which takes the union of several CAR-files.
use a shared z-frame cache, limited by byte size rather than pages.
use ManyCar in as many commands as possible.

Reference issue to close (if applicable)

Roadmap: #3222

Benchmarks

Linear scan through a mainnet snapshot to find the genesis block:

Command	Mean [s]	Min [s]	Max [s]	Relative
`Genesis Scan: 0-MiB`	7.829 ± 0.120	7.629	7.975	3.38 ± 0.06
`Genesis Scan: 32-MiB`	2.314 ± 0.019	2.287	2.354	1.00
`Genesis Scan: 64-MiB`	2.342 ± 0.031	2.316	2.401	1.01 ± 0.02
`Genesis Scan: 128-MiB`	2.401 ± 0.037	2.357	2.458	1.04 ± 0.02
`Genesis Scan: 256-MiB`	2.436 ± 0.019	2.400	2.473	1.05 ± 0.01
`Genesis Scan: 512-MiB`	2.522 ± 0.036	2.489	2.573	1.09 ± 0.02
`Genesis Scan: 1024-MiB`	2.542 ± 0.024	2.503	2.573	1.10 ± 0.01
`Genesis Scan: 2048-MiB`	2.523 ± 0.036	2.488	2.602	1.09 ± 0.02

Caching a single frame is sufficient when doing a linear scan. It's surprising that large caches hurt performance, though. Not sure why that is.

Executing 100 tipsets:

Command	Mean [s]	Min [s]	Max [s]	Relative
`Execute 100 tipsets: 0-MiB`	212.465 ± 1.082	211.468	213.615	3.33 ± 0.02
`Execute 100 tipsets: 32-MiB`	71.316 ± 1.105	70.204	72.415	1.12 ± 0.02
`Execute 100 tipsets: 64-MiB`	68.967 ± 0.652	68.378	69.667	1.08 ± 0.01
`Execute 100 tipsets: 128-MiB`	66.412 ± 0.174	66.265	66.604	1.04 ± 0.01
`Execute 100 tipsets: 256-MiB`	66.021 ± 0.080	65.947	66.106	1.04 ± 0.00
`Execute 100 tipsets: 512-MiB`	63.754 ± 0.270	63.504	64.039	1.00
`Execute 100 tipsets: 1024-MiB`	63.764 ± 0.357	63.391	64.102	1.00 ± 0.01
`Execute 100 tipsets: 2048-MiB`	64.399 ± 0.790	63.865	65.306	1.01 ± 0.01

Change checklist

I have performed a self-review of my own code,
I have made corresponding changes to the documentation,
I have added tests that prove my fix is effective or that my feature works (if possible),
I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

…nto lemmih/many-car

…press' into lemmih/many-car" This reverts commit 23034e3, reversing changes made to ab28415.

hanabi1224 · 2023-07-31T06:20:09Z

src/db/car/many.rs

+}
+
+impl<WriterT> ManyCar<WriterT> {
+    pub fn read_only<ReaderT: super::CarReader>(&mut self, any_car: AnyCar<ReaderT>) {


nit: push_read_only might be more descriptive

hanabi1224 · 2023-07-31T06:22:36Z

src/db/car/many.rs

+    writer: WriterT,
+}
+
+impl ManyCar {


nit: state migration tests could use ManyCar. I can also do that in a separate PR

nvm, bundle car is no longer needed once #3276 is merged

LesnyRumcajs

Adding subcommand tests, at least for the happy path, would be great.

lemmih · 2023-07-31T07:14:04Z

Adding subcommand tests, at least for the happy path, would be great.

I can add smoke tests once the --diff PR is merged.

LesnyRumcajs · 2023-07-31T07:21:23Z

Adding subcommand tests, at least for the happy path, would be great.

I can add smoke tests once the --diff PR is merged.

What's preventing us from doing a test without the --diff?

lemmih · 2023-07-31T07:36:01Z

Adding subcommand tests, at least for the happy path, would be great.

I can add smoke tests once the --diff PR is merged.

What's preventing us from doing a test without the --diff?

I don't have two CAR files to merge. :)

The primary use case for ManyCar is to merge a base snapshot and a set of diffs into a single, coherent block store. Without the --diff command, I'm not sure how to test it (other than the unit tests).

lemmih · 2023-07-31T09:04:47Z

Adding subcommand tests, at least for the happy path, would be great.

Done. Happy path tested.

LesnyRumcajs · 2023-07-31T09:37:59Z

@lemmih Sadly, the happy path is not so happy. https://github.com/ChainSafe/forest/actions/runs/5712759803/job/15477232048?pr=3288

lemmih · 2023-07-31T09:39:14Z

@lemmih Sadly, the happy path is not so happy. https://github.com/ChainSafe/forest/actions/runs/5712759803/job/15477232048?pr=3288

See!? This is why we should never add any tests! :P

LesnyRumcajs · 2023-07-31T10:22:30Z

src/db/car/forest.rs

+                // Frame cache hit, no value. This only happens when hashes collide
+                Some(None) => {}


Do we expect hash collisions? If not, should we panic here?

Hash collisions may occur, and it costs us little to handle them correctly. If two CIDs have the same hash, the index returns two possible positions. We check the second position if we don't find the correct CID at the first position.

lemmih added 27 commits July 27, 2023 08:08

snapshot compress: default output, progress bar, overwrite prompt

add4245

make spellcheck happy

413119c

move forest-car roots out of lock

ad4fff7

define heaviest_tipset for car blockstores

0cca482

basic many_car blockstore

fd288c7

use super-trait alias

f7ba299

Use a single cache in ManyCar

9400dbb

add license header

fe3f0cd

code clean up

1f03995

apply formatting

93f3bf5

document the AnyCar blockstore

96d2422

add more documentation

74d8874

add a few ManyCar tests

74af505

add tracking issue for parallel queries

0faee24

document the manycar store

0a2074d

use MemoryDB by default

5d2088e

limit the shared cache by bytes rather than pages

6175a85

more documentation, correctly compute cache size

4946feb

apply clippy suggestions

a286183

minor clean up

3452a07

word-wrap comments

872dd7f

yet more documentation

404b7c5

refactor code

bd02849

apply clippy suggestion

ab28415

Merge remote-tracking branch 'origin/lemmih/user-friendly-compress' i…

23034e3

…nto lemmih/many-car

make sure to drop lock guard

40baf91

Revert "Merge remote-tracking branch 'origin/lemmih/user-friendly-com…

ac63317

…press' into lemmih/many-car" This reverts commit 23034e3, reversing changes made to ab28415.

lemmih marked this pull request as ready for review July 31, 2023 05:35

lemmih requested a review from a team as a code owner July 31, 2023 05:35

lemmih removed the request for review from a team July 31, 2023 05:35

hanabi1224 reviewed Jul 31, 2023

View reviewed changes

hanabi1224 approved these changes Jul 31, 2023

View reviewed changes

LesnyRumcajs reviewed Jul 31, 2023

View reviewed changes

lemmih added 4 commits July 31, 2023 10:40

Merge branch 'main' into lemmih/many-car

f21e4f7

smoke test for taking the union of two car files

32a2544

apply formatting

6df00f9

76ca4cb

lemmih added 2 commits July 31, 2023 12:07

fix copy/paste mistake

05120f3

692a95d

LesnyRumcajs reviewed Jul 31, 2023

View reviewed changes

notice me senpai

6ec6551

LesnyRumcajs approved these changes Jul 31, 2023

View reviewed changes

lemmih added this pull request to the merge queue Jul 31, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 31, 2023

LesnyRumcajs added this pull request to the merge queue Jul 31, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 31, 2023

lemmih added this pull request to the merge queue Jul 31, 2023

Merged via the queue into main with commit 4edc2e4 Jul 31, 2023

lemmih deleted the lemmih/many-car branch July 31, 2023 12:48

This was referenced Jul 31, 2023

Cache multiple zstd frames in CompressedCarV1BackedBlockstore #3207

Closed

chore: reduce car cache from 2GiB to 1GiB #3310

Merged

sudo-shashank pushed a commit that referenced this pull request Aug 3, 2023

feat: add blockstore that is the union of several CAR stores (#3288)

6721770

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add blockstore that is the union of several CAR stores #3288

feat: add blockstore that is the union of several CAR stores #3288

lemmih commented Jul 30, 2023 •

edited

Loading

hanabi1224 Jul 31, 2023

hanabi1224 Jul 31, 2023

hanabi1224 Jul 31, 2023

LesnyRumcajs left a comment

lemmih commented Jul 31, 2023

LesnyRumcajs commented Jul 31, 2023

lemmih commented Jul 31, 2023

lemmih commented Jul 31, 2023

LesnyRumcajs commented Jul 31, 2023

lemmih commented Jul 31, 2023

LesnyRumcajs Jul 31, 2023

lemmih Jul 31, 2023

		// Frame cache hit, no value. This only happens when hashes collide
		Some(None) => {}

feat: add blockstore that is the union of several CAR stores #3288

feat: add blockstore that is the union of several CAR stores #3288

Conversation

lemmih commented Jul 30, 2023 • edited Loading

Summary of changes

Reference issue to close (if applicable)

Other information and links

Benchmarks

Change checklist

hanabi1224 Jul 31, 2023

Choose a reason for hiding this comment

hanabi1224 Jul 31, 2023

Choose a reason for hiding this comment

hanabi1224 Jul 31, 2023

Choose a reason for hiding this comment

LesnyRumcajs left a comment

Choose a reason for hiding this comment

lemmih commented Jul 31, 2023

LesnyRumcajs commented Jul 31, 2023

lemmih commented Jul 31, 2023

lemmih commented Jul 31, 2023

LesnyRumcajs commented Jul 31, 2023

lemmih commented Jul 31, 2023

LesnyRumcajs Jul 31, 2023

Choose a reason for hiding this comment

lemmih Jul 31, 2023

Choose a reason for hiding this comment

lemmih commented Jul 30, 2023 •

edited

Loading