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.

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

Include local MFS entity roots in Reprovider.Strategy=roots|pinned #10386

Open
lidel opened this issue Apr 3, 2024 · 1 comment · May be fixed by #10754
Open

Include local MFS entity roots in Reprovider.Strategy=roots|pinned #10386

lidel opened this issue Apr 3, 2024 · 1 comment · May be fixed by #10754
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up topic/MFS Topic MFS topic/provider Topic provider

Comments

@lidel
Copy link
Member

lidel commented Apr 3, 2024

Problem

Reprovider.Strategy settings roots and pinned are currently ignoring content implicitly pinned via MFS.

We start prioritizing pin announcements in all since #10376, and it is a pity MFS does not benefit from that. MFS gets announced with everyuthing else in cache.

Proposed improvement

(extracted from #10365 (comment))

Augment NewPinnedProvider and make it also walk the MFS (in offline mode, and skip content that is not in local repo), and include locally cached CIDs of "entities".

MFS walk should respect the 'onlyRoots' flag, and when set to true, only yield root CIDs of directories and root CIDs of files inside of them (as defined in IPIP-402)

cc https://github.com/ipshipyard/roadmaps/issues/8

@lidel lidel added kind/enhancement A net-new feature or improvement to an existing feature help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up topic/MFS Topic MFS topic/provider Topic provider exp/intermediate Prior experience is likely helpful effort/hours Estimated to take one or several hours labels Apr 3, 2024
@lidel lidel changed the title Include local MFS entity roots in Reprovider.Strategy Include local MFS entity roots in Reprovider.Strategy=roots|pinned Jan 13, 2025
@PsychoPunkSage
Copy link
Contributor

Hi @lidel
I tried to tackle this issue here #10704
I'm not sure if this is what is expected in the soln... please have a look at it.

hsanjuan added a commit that referenced this issue Mar 13, 2025
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).
@hsanjuan hsanjuan linked a pull request Mar 13, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up topic/MFS Topic MFS topic/provider Topic provider
Projects
No open projects
Status: No status
2 participants