Skip to content

Add MFS Support to Reprovider Strategy #10704

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Add MFS Support to Reprovider Strategy #10704

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e9477c0
fix::> Added local MFS rrots to pinned
PsychoPunkSage Feb 7, 2025
175f12d
Merge branch 'ipfs:master' into issue-10386
PsychoPunkSage Feb 7, 2025
b33097c
fix::> Added bstore checks
PsychoPunkSage Feb 7, 2025
5d130b3
fix::> cleanup + Added simple impl
PsychoPunkSage Feb 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions core/node/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

"github.com/ipfs/boxo/blockstore"
"github.com/ipfs/boxo/fetcher"
"github.com/ipfs/boxo/mfs"
pin "github.com/ipfs/boxo/pinning/pinner"
provider "github.com/ipfs/boxo/provider"
"github.com/ipfs/kubo/repo"
Expand Down Expand Up @@ -165,18 +166,24 @@
Pinner pin.Pinner
Blockstore blockstore.Blockstore
IPLDFetcher fetcher.Factory `name:"ipldFetcher"`
Root *mfs.Root
}
return func(in input) provider.KeyChanFunc {
if onlyRoots {
return provider.NewPinnedProvider(true, in.Pinner, in.IPLDFetcher)
}
pinnedProvider := provider.NewPinnedProvider(onlyRoots, in.Pinner, in.IPLDFetcher)

if onlyRoots || onlyPinned {
mfsRoot, err := in.Root.GetDirectory().GetNode()
if err != nil {
logger.Errorf("Error getting MFS root: %s", err)
return pinnedProvider
}

if onlyPinned {
return provider.NewPinnedProvider(false, in.Pinner, in.IPLDFetcher)
mfsProvider := provider.NewDAGProvider(mfsRoot.Cid(), in.IPLDFetcher)

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / interop-prep 

undefined: provider.NewDAGProvider

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / go-check 

undefined: provider.NewDAGProvider

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / go-check 

undefined: provider.NewDAGProvider

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / go-build 

undefined: provider.NewDAGProvider

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / go-lint 

undefined: provider.NewDAGProvider) (typecheck)

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / go-lint 

undefined: provider.NewDAGProvider) (typecheck)

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / go-lint 

undefined: provider.NewDAGProvider) (typecheck)

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / go-lint 

undefined: provider.NewDAGProvider (typecheck)

Check failure on line 181 in core/node/provider.go

View workflow job for this annotation

GitHub Actions / go-test 

undefined: provider.NewDAGProvider
Copy link
Member

@lidel lidel Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@PsychoPunkSage As noted by Hector above, this PR needs to include an end-to-end test in test/cli that ensures MFS provider is not prefetching any data that was not already in the local datastore.

Context:

  • MFS is different from local blockstore and local pins in that it is lazy-loaded.
    • I can do ipfs files cp /ipfs/bafybeie6tqsijf7j6gpow5q3qdjmtqgscrmd2ijl4olpxanbbptmvh7sui/ /Tsize-test and my node won't fetch 4EiB of data unless i start walking the dag.
    • People can ipfs files cp $(ipfs resolve -r /ipns/en.wikipedia-on-ipfs.org/) /wikipedia and use this to protect articles they visited from gc, without fetching entire thing.
    • We don't want providing to trigger fetch of any CIDs that are not already in local block-store.
  • Perhaps we should explicitly rename/replace in.IPLDFetcher here with in.OfflineIPLDFetcher? We already initialize both in ./core/node/core.go and store them separately to be extra sure offline one is used when it matters.
  • Separate concern: what happens once DAG walk errors on reference to a CID that is not in local store? We don't want to abort entire walk, but we should stop going deeper in this specific branch, and continue walking the DAG in other directions.

Copy link
Member

Choose a reason for hiding this comment

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

Triage notes:

return provider.NewPrioritizedProvider(pinnedProvider, mfsProvider)
}

return provider.NewPrioritizedProvider(
provider.NewPinnedProvider(true, in.Pinner, in.IPLDFetcher),
pinnedProvider,
provider.NewBlockstoreProvider(in.Blockstore),
)
}
Expand Down
Loading