Skip to content
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

Add MFS Support to Reprovider Strategy #10704

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

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 @@ import (

"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 @@ func newProvidingStrategy(onlyPinned, onlyRoots bool) interface{} {
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)
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.

return provider.NewPrioritizedProvider(pinnedProvider, mfsProvider)
}

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