Skip to content

frc-0108: f3-augmented-snapshot #1169

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

Merged

Conversation

hanabi1224
Copy link
Contributor

We propose extending the Filecoin CAR snapshot with an F3 snapshot as a raw data block, and changing CAR roots to be a CID that points to a CBOR-encoded Filecoin snapshot header struct.

@hanabi1224 hanabi1224 marked this pull request as ready for review June 25, 2025 08:23
@BigLep BigLep requested a review from masih June 26, 2025 14:33
@masih masih requested review from Kubuxu and removed request for masih June 26, 2025 14:36
@BigLep BigLep added this to F3 Jun 26, 2025
@github-project-automation github-project-automation bot moved this to Todo in F3 Jun 26, 2025
@BigLep BigLep moved this from Todo to In review in F3 Jun 26, 2025
BigLep added a commit that referenced this pull request Jun 26, 2025
This is an attempt to build upon #1169 with the key differences being:
1. SnapshotMetadata contains a version rather than F3SnapshotHeader.
2. F3Data is a cbor-encoded structure.
Copy link
Member

@BigLep BigLep left a comment

Choose a reason for hiding this comment

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

Thanks for starting on this @hanabi1224 . I know I wasn't really engaged in previous conversations, so apologies if any of my comments were covered during previous conversations. Feel free to point me to Google Doc comments, etc. These comments are coming from looking at this with fresh eyes.

My highest-level feeback items are:

  1. I think this FRC's scope should be "Filecoin Snapshots" which happens to include 2 versions. The focus should be on v2 (making snapshots F3 aware), but I think we need to specify enough of the old/v1 format.
  2. Why are we doing all the "varint" stuff vs. just relying on cbor encoding of the overall "f3data"?
  3. Why version the F3SnapshotHeader rather the SnapshotMetadata itself? That seems like the clear place to allow future extensibility even outside the "F3data" area.

For points 2 and 3, I created a commit drafting some alternative language: #1170 . I couldn't target my PR to your fork. If you like where that is going, maybe you can take that commit on top of the work in your fork?

And again, apologies if I'm wildly off or misunderstanding of things here...

### SnapshotMetadata

```go
type SnapshotMetadata {
Copy link
Member

Choose a reason for hiding this comment

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

Why don't we have a version field?

Copy link
Member

Choose a reason for hiding this comment

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

Good idea. We could do it extensibly so it's less painful as we go forward, see.

Suggested change
type SnapshotMetadata {
type SnapshotMetadata {
V1 SnapshotMetadataV1
}
type SnapshotMetadataV1 {

Then adding a V2 later makes it easy to support both versions; the top-level becomes a union where only one is present.

Copy link
Member

Choose a reason for hiding this comment

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

Not critical though, versioning can just be done by breaking the format and then checking schemas -- does this thing decode as a V1 format? No, then try V2. Just as long as they are different enough, like having a different number of fields.

@rvagg
Copy link
Member

rvagg commented Jun 30, 2025

As per comment @ https://github.com/filecoin-project/FIPs/pull/1169/files#r2170162191, we'll assign this one number 108. @hanabi1224 would you mind updating the various places the number needs to appear? filename, README, frontmatter, title.


```go
type SnapshotHeader struct {
Version uint64
Copy link
Member

Choose a reason for hiding this comment

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

See notes above about versioning, if we want to make it easier to version then we could multi-level this. Just a suggestion though. I don't have strong feelings either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great idea

@hanabi1224
Copy link
Contributor Author

hanabi1224 commented Jun 30, 2025

Why are we doing all the "varint" stuff vs. just relying on cbor encoding of the overall "f3data"?

@BigLep we proposed the CAR-like block structure to facilitate reading block streams from and writing block streams to the snapshot, with low RAM requirement. The current CBOR-gen tool seems to require holding all the blocks in memory for both reading and writing, please let me know if this is not a problem and it'd be great to point me to the best CBOR package that supports stream reading and writing

@hanabi1224 hanabi1224 changed the title frc: f3-augmented-snapshot frc-0108: f3-augmented-snapshot Jun 30, 2025
@BigLep
Copy link
Member

BigLep commented Jun 30, 2025

Why are we doing all the "varint" stuff vs. just relying on cbor encoding of the overall "f3data"?

@BigLep we proposed the CAR-like block structure to facilitate reading block streams from and writing block streams to the snapshot, with low RAM requirement. The current CBOR-gen tool seems to require holding all the blocks in memory for both reading and writing, please let me know if this is not a problem and it'd be great to point me to the best CBOR package that supports stream reading and writing

ACK - got it. I don't know CBOR libraries and which ones support streaming. I had done a quick search to see that it's possible, but I didn't investigate it beyond that and whether it applies in our context.

If we aren't packaging the whole thing in CBOR (fine for the reasons discussed) the rationale of this design decision seems important to capture in this document.

@rvagg
Copy link
Member

rvagg commented Jul 1, 2025

I'm assuming that we're just talking about standard CAR sections for these new blocks, with varint length prefixes as per CAR format. If so, as per @BigLep's confusion, I wonder if it's worth either clarifying that this is what we're talking about ("with varint-encoded length prefix as per the CAR format"), or even just remove mention of the prefixes entirely since it's really just an implementation detail of the CAR format that we don't need to duplicate here.

Essentially you're just turning "Header block" and "Data block" into things within the CAR. Then, beyond that, for the consumer of this data the important part is distinguishing between F3 data and non-F3 data.

Which brings me to the next question I have when I look at this - how do I distinguish? For Lotus, we're going to want to dump the non-F3 data into the blockstore like we currently do, and then shunt the F3 data off into a separate store. I can iterate through the CAR in any number of ways (including streaming large blocks -- please don't feel we are limited by cbor-gen, or the current way we use go-car, we have variations of these things, which also include streaming large blocks to avoid pre-allocation). But how will I know what to put where? Do I have to read the F3 header, then count instance indexes, or does the end-start give me the N I need? Or do I need to do the reverse and traverse the chain DAG and then everything that's not in that DAG is assumed to be F3 data? It would be good if you could make it clear how a consumer of this data sorts between them.

@hanabi1224
Copy link
Contributor Author

hanabi1224 commented Jul 1, 2025

I wonder if it's worth either clarifying that this is what we're talking about ("with varint-encoded length prefix as per the CAR format"), or even just remove mention of the prefixes entirely since it's really just an implementation detail of the CAR format that we don't need to duplicate here.

@rvagg updated and clarified that the F3 snapshot is in a CARv1-like format but not CAR (data blocks are not content addressed by CIDs). All prefixes thing mentioned in the FRC spec details of the F3 snapshot format

how do I distinguish?

The F3 data CID is defined in the SnapshotMetadata to distinguish from the chain IPLD blocks. The F3 data block should be just delegated entirely as an io.Reader to the F3 package for importing. A draft implementation: filecoin-project/go-f3#1032

@hanabi1224 hanabi1224 force-pushed the hm/frc-f3-augmented-snapshot branch from ede57af to 78614ae Compare July 1, 2025 13:30
@hanabi1224
Copy link
Contributor Author

hanabi1224 commented Jul 1, 2025

Why version the F3SnapshotHeader rather the SnapshotMetadata itself? That seems like the clear place to allow future extensibility even outside the "F3data" area.

@BigLep I agree that we should version SnapshotMetadata. At the same time, I think the version of F3SnapshotHeader is needed as well. The F3 data in meant to be handled by the F3 packages (go-f3) and a Filecoin node treats it as a black box, any changes in the F3 data format should be transparent to Filecoin node implementations and should not change the FilecoinSnapshotMetadata version, does that make sense?

FRCs/frc-0108.md Outdated
Comment on lines 231 to 233
Nodes starting from a snapshot should not rely on the certificate exchange protocol to catch up with the F3 data because we expect this will get slower over time. A slow F3 catchup time leads to, e.g.

- delay in the readiness of F3-aware RPC APIs
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Nodes starting from a snapshot should not rely on the certificate exchange protocol to catch up with the F3 data because we expect this will get slower over time. A slow F3 catchup time leads to, e.g.
- delay in the readiness of F3-aware RPC APIs
### Start-up without initial F3 data
Nodes starting from a snapshot should not rely on the certificate exchange protocol to catch up with the F3 data because we expect this will get slower over time. One outcome of a slow F3 catchup time is a delay in the readiness of F3-aware RPC APIs.
### CAR format expectations
This change introduces a relatively novel use of the CAR format in that it contains one very large block, much larger than typical blocks found in most CAR containers for use with IPLD data. At the time of this proposal, this block size would be approximately TODO MiB and this will only grow over time. While this is not disallowed by the CAR specification, many CAR processing utilities are built on an assumption of classic IPFS style blocks of more more than approximately 1MiB each. Some CAR tooling may struggle to deal with the new proposed format, although handling CAR data outside of the narrow use-case of snapshort imports on Filecoin nodes is not typical or necessarily recommended.

See TODO in there, I don't know what size we're talking about here, do you have a rough guestimate to get us started? If it's too hard we could remove the specific reference but I wanted to provide a ballpark to point out how it's different to the max ~1MiB expectation that a lot of tools expect today. But also be clear that this isn't necessarily a bad thing, just bad expectations in existing software.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rvagg filled TODO with 100MiB. The current F3 db size on mainnet is ~100MiB in ~2 month. We do plan to extend the current CAR package with API that returns io.Reader for the data block to avoid loading the entire F3 data into RAM

@rvagg
Copy link
Member

rvagg commented Jul 2, 2025

This reads really well now and it's pretty clear. Thanks for the clarification of "CARv1-like".
I've left some suggestions inline but otherwise this seems good to merge to me.

Copy link
Member

@BigLep BigLep left a comment

Choose a reason for hiding this comment

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

Thanks @hanabi1224 for the quick turn around!

FYI that I did a few small cosmetic things in 7dac2e2 . I moved the V1 specification before the V2 specification and I did more section linking.

I also documented my understanding of some of the design considerations/decisions in 11a7291.

(Please change anything I have wrong. I'm fully expecting these commits to get squashed or removed...)

Concerning versioning F3SnapshotHeader as well, that seems fine to add as well.

@hanabi1224 hanabi1224 force-pushed the hm/frc-f3-augmented-snapshot branch from 9641c31 to c39d824 Compare July 2, 2025 12:16
@hanabi1224 hanabi1224 force-pushed the hm/frc-f3-augmented-snapshot branch from c39d824 to 412adf0 Compare July 2, 2025 12:18
@hanabi1224
Copy link
Contributor Author

hanabi1224 commented Jul 2, 2025

FYI that I did a few small cosmetic things

@BigLep Thanks a lot!

Concerning versioning F3SnapshotHeader as well,

@BigLep design concerns are added in 8db56b3.

@BigLep BigLep requested a review from rvagg July 2, 2025 18:08
@BigLep
Copy link
Member

BigLep commented Jul 2, 2025

Looks good to me @hanabi1224 . Once we get the official approval from @rvagg , we can work to get it merged

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

lgtm!

@BigLep
Copy link
Member

BigLep commented Jul 3, 2025

I'm going to squash and merge as this is a draft. v1 is already implemented but as this doc is primarily focused on v2, we'll change the status out of draft once there is a v2 implementation.

@BigLep BigLep merged commit f91cbe1 into filecoin-project:master Jul 3, 2025
1 check passed
@github-project-automation github-project-automation bot moved this from In review to Done in F3 Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants