Skip to content

Commit 846e219

Browse files
hsanjuanlidel
andauthored
feat: Reprovider.Strategy: rename "flat" to "all" (#10928)
* Reprovider strategy: rename "flat" to "all". Value "flat" now parses to "all". Behaviour from "all" removed. Fixes #10864 which has detailed explanation. * core/node/provider.go: remove unused function mfsRootProvider It was used in the "all" strategy. * docs: improve reprovider.strategy=all changelog framing - highlight memory efficiency improvements - clarify this removes v0.28 workaround - update config.md memory requirements - fix announce-on profile typo * feat: deprecate Reprovider.Strategy=flat - 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 --------- Co-authored-by: Marcin Rataj <[email protected]>
1 parent 426477e commit 846e219

File tree

9 files changed

+54
-81
lines changed

9 files changed

+54
-81
lines changed

cmd/ipfs/kubo/daemon.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,10 @@ func daemonFunc(req *cmds.Request, re cmds.ResponseEmitter, env cmds.Environment
479479
if cfg.Provider.Strategy.WithDefault("") != "" && cfg.Reprovider.Strategy.IsDefault() {
480480
log.Fatal("Invalid config. Remove unused Provider.Strategy and set Reprovider.Strategy instead. Documentation: https://github.com/ipfs/kubo/blob/master/docs/config.md#reproviderstrategy")
481481
}
482+
// Check for deprecated "flat" strategy
483+
if cfg.Reprovider.Strategy.WithDefault("") == "flat" {
484+
log.Error("Reprovider.Strategy='flat' is deprecated and will be removed in the next release. Please update your config to use 'all' instead.")
485+
}
482486
if cfg.Experimental.StrategicProviding {
483487
log.Error("Experimental.StrategicProviding was removed. Remove it from your config and set Provider.Enabled=false to remove this message. Documentation: https://github.com/ipfs/kubo/blob/master/docs/experimental-features.md#strategic-providing")
484488
cfg.Experimental.StrategicProviding = false

config/reprovider.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,10 @@ const (
1313
type ReproviderStrategy int
1414

1515
const (
16-
ReproviderStrategyAll ReproviderStrategy = 1 << iota // 1 (0b00001)
17-
ReproviderStrategyFlat // 2 (0b00010)
18-
ReproviderStrategyPinned // 4 (0b00100)
19-
ReproviderStrategyRoots // 8 (0b01000)
20-
ReproviderStrategyMFS // 16 (0b10000)
16+
ReproviderStrategyAll ReproviderStrategy = 1 << iota
17+
ReproviderStrategyPinned
18+
ReproviderStrategyRoots
19+
ReproviderStrategyMFS
2120
)
2221

2322
// Reprovider configuration describes how CID from local datastore are periodically re-announced to routing systems.
@@ -31,10 +30,8 @@ func ParseReproviderStrategy(s string) ReproviderStrategy {
3130
var strategy ReproviderStrategy
3231
for _, part := range strings.Split(s, "+") {
3332
switch part {
34-
case "all", "": // special case, does not mix with others
33+
case "all", "flat", "": // special case, does not mix with others ("flat" is deprecated, maps to "all")
3534
return ReproviderStrategyAll
36-
case "flat":
37-
strategy |= ReproviderStrategyFlat
3835
case "pinned":
3936
strategy |= ReproviderStrategyPinned
4037
case "roots":

config/reprovider_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package config
2+
3+
import "testing"
4+
5+
func TestParseReproviderStrategy(t *testing.T) {
6+
tests := []struct {
7+
input string
8+
expect ReproviderStrategy
9+
}{
10+
{"all", ReproviderStrategyAll},
11+
{"pinned", ReproviderStrategyPinned},
12+
{"mfs", ReproviderStrategyMFS},
13+
{"pinned+mfs", ReproviderStrategyPinned | ReproviderStrategyMFS},
14+
{"invalid", 0},
15+
{"all+invalid", ReproviderStrategyAll},
16+
{"", ReproviderStrategyAll},
17+
{"flat", ReproviderStrategyAll}, // deprecated, maps to "all"
18+
{"flat+all", ReproviderStrategyAll},
19+
}
20+
21+
for _, tt := range tests {
22+
result := ParseReproviderStrategy(tt.input)
23+
if result != tt.expect {
24+
t.Errorf("ParseReproviderStrategy(%q) = %d, want %d", tt.input, result, tt.expect)
25+
}
26+
}
27+
}

core/coreapi/unixfs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func (api *UnixfsAPI) Add(ctx context.Context, files files.Node, opts ...options
108108

109109
// wrap the DAGService in a providingDAG service which provides every block written.
110110
// note about strategies:
111-
// - "all"/"flat" gets handled directly at the blockstore so no need to provide
111+
// - "all" gets handled directly at the blockstore so no need to provide
112112
// - "roots" gets handled in the pinner
113113
// - "mfs" gets handled in mfs
114114
// We need to provide the "pinned" cases only. Added blocks are not

core/node/provider.go

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -184,19 +184,6 @@ func mfsProvider(mfsRoot *mfs.Root, fetcher fetcher.Factory) provider.KeyChanFun
184184
}
185185
}
186186

187-
func mfsRootProvider(mfsRoot *mfs.Root) provider.KeyChanFunc {
188-
return func(ctx context.Context) (<-chan cid.Cid, error) {
189-
rootNode, err := mfsRoot.GetDirectory().GetNode()
190-
if err != nil {
191-
return nil, fmt.Errorf("error loading mfs root, cannot provide MFS: %w", err)
192-
}
193-
ch := make(chan cid.Cid, 1)
194-
ch <- rootNode.Cid()
195-
close(ch)
196-
return ch, nil
197-
}
198-
}
199-
200187
type provStrategyIn struct {
201188
fx.In
202189
Pinner pin.Pinner
@@ -219,8 +206,7 @@ type provStrategyOut struct {
219206
// - "roots": Only root CIDs of pinned content
220207
// - "pinned": All pinned content (roots + children)
221208
// - "mfs": Only MFS content
222-
// - "flat": All blocks, no prioritization
223-
// - "all": Prioritized: pins first, then MFS roots, then all blocks
209+
// - "all": all blocks
224210
func createKeyProvider(strategyFlag config.ReproviderStrategy, in provStrategyIn) provider.KeyChanFunc {
225211
switch strategyFlag {
226212
case config.ReproviderStrategyRoots:
@@ -234,28 +220,11 @@ func createKeyProvider(strategyFlag config.ReproviderStrategy, in provStrategyIn
234220
)
235221
case config.ReproviderStrategyMFS:
236222
return mfsProvider(in.MFSRoot, in.OfflineUnixFSFetcher)
237-
case config.ReproviderStrategyFlat:
223+
default: // "all", "", "flat" (compat)
238224
return in.Blockstore.AllKeysChan
239-
default: // "all", ""
240-
return createAllStrategyProvider(in)
241225
}
242226
}
243227

244-
// createAllStrategyProvider creates the complex "all" strategy provider.
245-
// This implements a three-tier priority system:
246-
// 1. Root blocks of direct and recursive pins (highest priority)
247-
// 2. MFS root (medium priority)
248-
// 3. All other blocks in blockstore (lowest priority)
249-
func createAllStrategyProvider(in provStrategyIn) provider.KeyChanFunc {
250-
return provider.NewPrioritizedProvider(
251-
provider.NewPrioritizedProvider(
252-
provider.NewBufferedProvider(dspinner.NewPinnedProvider(true, in.Pinner, in.OfflineIPLDFetcher)),
253-
mfsRootProvider(in.MFSRoot),
254-
),
255-
in.Blockstore.AllKeysChan,
256-
)
257-
}
258-
259228
// detectStrategyChange checks if the reproviding strategy has changed from what's persisted.
260229
// Returns: (previousStrategy, hasChanged, error)
261230
func detectStrategyChange(ctx context.Context, strategy string, ds datastore.Datastore) (string, bool, error) {

core/node/storage.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,12 @@ func BaseBlockstoreCtor(
3939
opts := []blockstore.Option{blockstore.WriteThrough(writeThrough)}
4040

4141
// Blockstore providing integration:
42-
// When strategy includes "all" or "flat", the blockstore directly provides blocks as they're Put.
42+
// When strategy includes "all" the blockstore directly provides blocks as they're Put.
4343
// Important: Provide calls from blockstore are intentionally BLOCKING.
4444
// The Provider implementation (not the blockstore) should handle concurrency/queuing.
4545
// This avoids spawning unbounded goroutines for concurrent block additions.
4646
strategyFlag := config.ParseReproviderStrategy(providingStrategy)
47-
shouldProvide := config.ReproviderStrategyAll | config.ReproviderStrategyFlat
48-
if strategyFlag&shouldProvide != 0 {
47+
if strategyFlag&config.ReproviderStrategyAll != 0 {
4948
opts = append(opts, blockstore.Provider(prov))
5049
}
5150

docs/changelogs/v0.37.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ This release was brought to you by the [Shipyard](https://ipshipyard.com/) team.
1919
- [📌 Named pins in `ipfs add` command](#-named-pins-in-ipfs-add-command)
2020
- [Custom sequence numbers in `ipfs name publish`](#custom-sequence-numbers-in-ipfs-name-publish)
2121
- [⚙️ `Reprovider.Strategy` is now consistently respected](#-reprovider-strategy-is-now-consistently-respected)
22+
- [⚙️ `Reprovider.Strategy=all`: improved memory efficiency](#-reproviderstrategyall-improved-memory-efficiency)
2223
- [Removed unnecessary dependencies](#removed-unnecessary-dependencies)
2324
- [Improved `ipfs cid`](#improved-ipfs-cid)
2425
- [Deprecated `ipfs stats reprovide`](#deprecated-ipfs-stats-reprovide)
@@ -151,7 +152,7 @@ $ ipfs pin ls --names
151152
bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi recursive testname
152153
```
153154

154-
#### ⚙️ `Reprovider.Strategy` is now consistently respected
155+
#### ⚙️ `Reprovider.Strategy` is now consistently respected.
155156

156157
Prior to this version, files added, blocks received etc. were "provided" to the network (announced on the DHT) regardless of the ["reproviding strategy" setting](https://github.com/ipfs/kubo/blob/master/docs/config.md#reproviderstrategy). For example:
157158

@@ -165,6 +166,15 @@ This was inefficient as content that should not be provided was getting provided
165166
> [!NOTE]
166167
> **Behavior change:** The `--offline` flag no longer affects providing behavior. Both `ipfs add` and `ipfs --offline add` now provide blocks according to the reproviding strategy when run against an online daemon (previously `--offline add` did not provide). Since `ipfs add` has been nearly as fast as offline mode [since v0.35](https://github.com/ipfs/kubo/blob/master/docs/changelogs/v0.35.md#fast-ipfs-add-in-online-mode), `--offline` is rarely needed. To run truly offline operations, use `ipfs --offline daemon`.
167168
169+
#### ⚙️ `Reprovider.Strategy=all`: improved memory efficiency
170+
171+
The memory cost of `Reprovider.Strategy=all` no longer grows with the number of pins. The strategy now processes blocks directly from the datastore in undefined order, eliminating the memory pressure tied to the number of pins.
172+
173+
As part of this improvement, the `flat` reprovider strategy has been renamed to `all` (the default). This cleanup removes the workaround introduced in v0.28 for pin root prioritization. With the introduction of more granular strategies like [`pinned+mfs`](https://github.com/ipfs/kubo/blob/master/docs/config.md#reproviderstrategy), we can now optimize the default `all` strategy for lower memory usage without compromising users who need pin root prioritization ([rationale](https://github.com/ipfs/kubo/pull/10928#issuecomment-3211040182)).
174+
175+
> [!NOTE]
176+
> **Migration guidance:** If you experience undesired announcement delays of root CIDs with the new `all` strategy, switch to `pinned+mfs` for root prioritization.
177+
168178
#### Removed unnecessary dependencies
169179

170180
Kubo has been cleaned up by removing unnecessary dependencies and packages:

docs/config.md

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,7 +2063,6 @@ Type: `optionalDuration` (unset for the default)
20632063
Tells reprovider what should be announced. Valid strategies are:
20642064

20652065
- `"all"` - announce all CIDs of stored blocks
2066-
- Order: root blocks of direct and recursive pins and MFS root are announced first, then the rest of blockstore
20672066
- `"pinned"` - only announce recursively pinned CIDs (`ipfs pin add -r`, both roots and child blocks)
20682067
- Order: root blocks of direct and recursive pins are announced first, then the child blocks of recursive pins
20692068
- `"roots"` - only announce the root block of explicitly pinned CIDs (`ipfs pin add`)
@@ -2079,15 +2078,13 @@ Tells reprovider what should be announced. Valid strategies are:
20792078
- `"pinned+mfs"` - a combination of the `pinned` and `mfs` strategies.
20802079
- **ℹ️ NOTE:** This is the suggested strategy for users who run without GC and don't want to provide everything in cache.
20812080
- Order: first `pinned` and then the locally available part of `mfs`.
2082-
- `"flat"` - same as `all`, announce all CIDs of stored blocks, but without prioritizing anything.
20832081

20842082
**Strategy changes automatically clear the provide queue.** When you change `Reprovider.Strategy` and restart Kubo, the provide queue is automatically cleared to ensure only content matching your new strategy is announced. You can also manually clear the queue using `ipfs provide clear`.
20852083

20862084
**Memory requirements:**
20872085

2088-
- Reproviding larger pinsets using the `all`, `mfs`, `pinned`, `pinned+mfs` or `roots` strategies requires additional memory, with an estimated ~1 GiB of RAM per 20 million items for reproviding to the Amino DHT.
2086+
- Reproviding larger pinsets using the `mfs`, `pinned`, `pinned+mfs` or `roots` strategies requires additional memory, with an estimated ~1 GiB of RAM per 20 million items for reproviding to the Amino DHT.
20892087
- This is due to the use of a buffered provider, which avoids holding a lock on the entire pinset during the reprovide cycle.
2090-
- The `flat` strategy can be used to lower memory requirements, but only recommended if memory utilization is too high, prioritization of pins is not necessary, and it is acceptable to announce every block cached in the local repository.
20912088

20922089
Default: `"all"`
20932090

@@ -3446,7 +3443,7 @@ Disables [Reprovider](#reprovider) system (and announcing to Amino DHT).
34463443
34473444
### `announce-on` profile
34483445

3449-
(Re-)enables [Reprovider](#reprovider) system (reverts [`announce-off` profile](#annouce-off-profile).
3446+
(Re-)enables [Reprovider](#reprovider) system (reverts [`announce-off` profile](#announce-off-profile)).
34503447

34513448
### `legacy-cid-v0` profile
34523449

test/cli/provider_test.go

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -187,18 +187,6 @@ func TestProvider(t *testing.T) {
187187
expectProviders(t, cid, nodes[0].PeerID().String(), nodes[1:]...)
188188
})
189189

190-
t.Run("Provide with 'flat' strategy", func(t *testing.T) {
191-
t.Parallel()
192-
193-
nodes := initNodes(t, 2, func(n *harness.Node) {
194-
n.SetIPFSConfig("Reprovider.Strategy", "flat")
195-
})
196-
defer nodes.StopDaemons()
197-
198-
cid := nodes[0].IPFSAddStr("flat strategy")
199-
expectProviders(t, cid, nodes[0].PeerID().String(), nodes[1:]...)
200-
})
201-
202190
t.Run("Provide with 'pinned' strategy", func(t *testing.T) {
203191
t.Parallel()
204192

@@ -308,24 +296,6 @@ func TestProvider(t *testing.T) {
308296
expectProviders(t, cid, nodes[0].PeerID().String(), nodes[1:]...)
309297
})
310298

311-
t.Run("Reprovides with 'flat' strategy", func(t *testing.T) {
312-
t.Parallel()
313-
314-
nodes := initNodesWithoutStart(t, 2, func(n *harness.Node) {
315-
n.SetIPFSConfig("Reprovider.Strategy", "flat")
316-
})
317-
318-
cid := nodes[0].IPFSAddStr(time.Now().String())
319-
320-
nodes = nodes.StartDaemons().Connect()
321-
defer nodes.StopDaemons()
322-
expectNoProviders(t, cid, nodes[1:]...)
323-
324-
nodes[0].IPFS("routing", "reprovide")
325-
326-
expectProviders(t, cid, nodes[0].PeerID().String(), nodes[1:]...)
327-
})
328-
329299
t.Run("Reprovides with 'pinned' strategy", func(t *testing.T) {
330300
t.Parallel()
331301

0 commit comments

Comments
 (0)