provider: provide MFS DAG using offline fetcher. #10754
Conversation
Fixes #10386. Overseeds #10704. This augments the current provider KeyChanFuncs with the CIDs coming from the MFS DAG. MFS CIDs are listed using the offline fetcher, which has been updated with SkipNotFound so that no errors happen during traversal when nodes are missing. Strategies have been updated: - For "roots" and "all" strategy we announce the MFS root only, after the pins. - For the "pinned" strategy, we announce the MFS Cids after the pin cids. - A new "mfs" strategy is added, only for MFS cids. The code around strategy selection has been slightly change for the better (I hope).
| case "pinned": | ||
| return provider.NewPrioritizedProvider( | ||
| provider.NewBufferedProvider(provider.NewPinnedProvider(false, in.Pinner, in.IPLDFetcher)), | ||
| mfsProvider(in.MFSRoot, in.OfflineUnixFSFetcher), |
There was a problem hiding this comment.
The downside here is that there is no strategy that only provide "recursive-pinned" content, and no mfs.
If we don't provide mfs here, there will be no policy that provides only "recursive-pinned+mfs" content... but it could be added as pinned+mfs or so...
There was a problem hiding this comment.
MFS is implicitly pinned (protected from GC) so I think it is fine to keep it under pinned.
But things get hairy with roots (we dont have ability to only announce dir and file roots as we walk MFS, right?).
Maybe keeping pinned and roots as-is, and adding separate pinned+mfs for now is the right way of handling this? (we dont change behavior for existing users, and new users can choose pinned+mfs).
There was a problem hiding this comment.
Is it a problem to only announce the MFS-root? (consider it a big file)... Or you mean this could cause misunderstandings?
There was a problem hiding this comment.
in the meantime I restored "pinned" and "roots" and added "pinned+mfs"
There was a problem hiding this comment.
I'm thinking:
- if you pin something then that pin is usually recursive
pinnedannounces recursive pins by walking the entire DAG behind each recursively pinned CIDrootsannounces only root cids of pins (no DAG walk at all)
pinned+mfsshould do the same aspinned– walk the MFS DAG and announce MFS recursively (only local blocks).roots- easy: could also announce only MFS root (no need to create
roots+mfsjust for one CID, fine to announce MFS root with other pin roots) - harder: make MFS walk smart enough to only yield root CIDs of files and directories (imo this comes with extra overhead, and no longer can be
rootsand requires separate profileroots+mfs)
- easy: could also announce only MFS root (no need to create
I think its fine to do "easy" roots for now – announce the MFS root for now (without walking dag and discovering sub-entity roots).
We can add roots+mfs if users ask for it.
There was a problem hiding this comment.
could also announce only MFS root (no need to create roots+mfs just for one CID, fine to announce MFS root with other pin roots)
This is how it was.
|
Looking good. Updated docs + ipfs/boxo#847 and started local test of |
|
Cosmetic: providing MFS which does not have all blocks locally, prints this to logs: the specific "skip" event should not be logged as error (ok to log it as DEBUG) |
Just to check, you removed the root MFS cid? It is the only way I can reproduce this. |
|
And if the root block is not there, maybe it should be logged as error? It means we have initiated a traversal from a block that is not there which is probably not intended |
|
@hsanjuan no, I have MFS root. Perhaps this "skip" is due to one of partial dags I have (e.g. Lmk if this is not enough, I could block some time to debug try to reproduce with smaller repo if that helps. |
|
@lidel the only way I get the dagprovider to spit out the error is when the root is missing. Walking partial DAGs should not cause it. Otherwise it would indicate that we are aborting partial-dag walks, which we shouldn't. I have updated boxo to log the root cid when this error is produced. Can you test again? We can then decide at least if this root cid belongs to mfs or to a specific DAG or to something else. |
Fixes #10386. Overseeds #10704. Part of https://github.com/ipshipyard/roadmaps/issues/8
This augments the current provider KeyChanFuncs with the CIDs coming from the MFS DAG. MFS CIDs are listed using the offline fetcher, which has been updated with SkipNotFound so that no errors happen during traversal when nodes are missing.
Strategies have been updated:
The code around strategy selection has been slightly change for the better (I hope).