Skip to content

feat: Reprovider.Strategy: rename "flat" to "all". #10928

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

Jump to bottom
Merged
merged 5 commits into from
Aug 21, 2025
Merged

feat: Reprovider.Strategy: rename "flat" to "all". #10928

merged 5 commits into from
Aug 21, 2025

Conversation

@hsanjuan
Copy link
Contributor

@hsanjuan hsanjuan commented Aug 21, 2025

Value "flat" now parses to "all". Behaviour from "all" removed.

Fixes #10864 which has detailed explanation.

@hsanjuan
Reprovider strategy: rename "flat" to "all".
8472641 
Value "flat" now parses to "all". Behaviour from "all" removed.

Fixes #10864 which has detailed explanation.
@hsanjuan hsanjuan self-assigned this Aug 21, 2025
@hsanjuan hsanjuan requested a review from a team as a code owner August 21, 2025 13:40
@hsanjuan
Copy link
Contributor Author

hsanjuan commented Aug 21, 2025

@lidel I put this in the 0.37 changelog, assuming it can still get into that release.

@lidel lidel assigned lidel and unassigned hsanjuan Aug 21, 2025
@lidel lidel changed the title Reprovider strategy: rename "flat" to "all". feat: Reprovider.Strategy: rename "flat" to "all". Aug 21, 2025
guillaumemichel
guillaumemichel approved these changes Aug 21, 2025
View reviewed changes
Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

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

LGTM!

Linter seems unhappy about an unused function.

@lidel
Copy link
Member

lidel commented Aug 21, 2025

Writing down rationale why we added this and now removing it:

  • The behavior of all in 0.36 prioritizes pin roots before announcing everything else. this was introduced as a workaround in:
  • Issue: Prioritize announcement of pin roots #10365
  • TLDR
    • 👉 If we remove this optimization, Reprovider.Strategy=all users who are falling behind announcements will effectively see a regression: before, content (identified by ROOT cids) had providers, and now, % of it may not have because who knows when the root gets announced.
    • 👉 this is not a problem, because those users now can switch to Reprovider.Strategy=pinned+mfs (or roots) that do the root priotization and only announce pinned content
    • (i'll update changelog to make this more clear)

lidel added 2 commits August 21, 2025 18:21
@lidel
docs: improve reprovider.strategy=all changelog framing
abea1c4 
- highlight memory efficiency improvements
- clarify this removes v0.28 workaround
- update config.md memory requirements
- fix announce-on profile typo
@lidel
feat: deprecate Reprovider.Strategy=flat
28a84e4 
- add deprecation warning in daemon.go when flat strategy is detected
- document that flat is deprecated in ParseReproviderStrategy comment
- add explicit test case for flat -> all mapping
- flat continues to work but users are warned to migrate to all
@lidel lidel merged commit 846e219 into master Aug 21, 2025
16 of 17 checks passed
@lidel lidel deleted the fix/10864-merge-reprovide-all-flat branch August 21, 2025 17:02
@lidel lidel mentioned this pull request Aug 11, 2025
Open
@BrewTestBot BrewTestBot mentioned this pull request Aug 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lidel lidel lidel approved these changes

@guillaumemichel guillaumemichel guillaumemichel approved these changes

Assignees

@lidel lidel

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Merge "all" and "flat" reprovider strategies

4 participants

@hsanjuan @lidel @guillaumemichel