feat: `Provider.WorkerCount` and `stats reprovide` cmd #10779

guillaumemichel · 2025-04-09T12:46:57Z

This PR finalizes separation of provide and reprovide queues and exposes new configuration for adjusting performance of first-time provide.

Companion to provider: dedicated provide queue boxo#907
Part of Improve the providing subsystem in Kubo — IPFS/2025 ipshipyard/roadmaps#6
Closes Provider performance regression #10777

guillaumemichel · 2025-04-16T11:17:14Z

Currently two failing tests:

--- FAIL: TestIPFSVersionDeps (0.11s)
    basic_commands_test.go:73: 
        	Error Trace:	github.com/ipfs/kubo/test/cli/basic_commands_test.go:73
        	Error:      	Received unexpected error:
        	           	malformed module path "../boxo": invalid path element ".."
        	Test:       	TestIPFSVersionDeps
        	Messages:   	path: ../boxo, version: (devel)
2025-04-16T13:11:19.477+0200	PANIC	testharness	harness/run.go:128	'[/home/guissou/Documents/ipfs/kubo/cmd/ipfs/ipfs add -q --max-file-links 1024]' returned error, code: 1, err: exit status 1
stdout:
stderr:Error: unknown option "max-file-links"


--- FAIL: TestAdd (4.24s)
    --- FAIL: TestAdd/produced_unixfs_max_file_links:_command_flag_--max-file-links_overrides_configuration_in_Import.UnixFSFileMaxLinks (0.52s)
panic: '[/home/guissou/Documents/ipfs/kubo/cmd/ipfs/ipfs add -q --max-file-links 1024]' returned error, code: 1, err: exit status 1
	stdout:
	stderr:Error: unknown option "max-file-links"
	
	[recovered]
	panic: '[/home/guissou/Documents/ipfs/kubo/cmd/ipfs/ipfs add -q --max-file-links 1024]' returned error, code: 1, err: exit status 1
	stdout:
	stderr:Error: unknown option "max-file-links"

lidel

Thank you @guillaumemichel, quick pass from UX perspective with some asks inline

config/provider.go

lidel · 2025-04-27T17:37:08Z

core/commands/stat_provide.go

nit: renaming field names here is a breaking change to existing command.

It will break compatibility with older RPC clients, libraries, and will also break user's scripts and automations.

Mind adding a note about this to the Changelog?

Since we have two queues, and they behave differently, I wonder if we should have ipfs stat provide and ipfs stat reprovide? (e.g. if we make breaking change anyway, any reason to not rename current provide to reprovide?)

I marked ipfs stats provide as deprecated, and renamed it to ipfs stats reprovide.

ipfs stats provide shows the same stats as ipfs stats reprovide but with the old names (only information related to reprovides, because we don't track provides anymore), so it shouldn't break existing scripts.

docs/changelogs/v0.35.md

docs/config.md

we dont do that for Reprovider.Strategy which is more important, this should reduce user confusion

explicitly document it is not used - without this legacy users will have it in their config and be very confused

lidel

configuration lgtm, thank you!

(pushed small cosmetic tweaks to reduce confusion related to Provider.Strategy vs Reprovider.Strategy)

gammazero · 2025-04-30T13:27:15Z

core/commands/stat_provide.go

+			fmt.Fprintf(wtr, "TotalProvides:\t%s\n", humanNumber(s.TotalReprovides))
+			fmt.Fprintf(wtr, "AvgProvideDuration:\t%s\n", humanDuration(s.AvgReprovideDuration))


Are the printed names correct? Seems like they should match the variable names, even if that causes a breaking change.

Well, we would be making a breaking change – see #10779 (comment)

Safer to keep stats provide as-is now and mark it as deprecated + add new stats reprovide which has new labels.

We will remove / change stats provide in future release.

gammazero · 2025-04-30T13:31:10Z

docs/config.md

+
+### `Provider.Strategy`
+
+Legacy, not used at the moment, see [`Reprovider.Strategy`](#reproviderstrategy) instead.


Suggested change

Legacy, not used at the moment, see [`Reprovider.Strategy`](#reproviderstrategy) instead.

Legacy, no longer used, see [`Reprovider.Strategy`](#reproviderstrategy) instead.

Otherwise, it sounds like we are trying to communicate a planned future use.

We might? Now that we have separate queues, we may have strategies related to provide that are different than reprovide

Left as-is for now.

guillaumemichel force-pushed the adjust-stats-provide branch from 7ec2b65 to da10f2e Compare April 9, 2025 14:10

adjust ipfs stats provide

05d0bb3

guillaumemichel force-pushed the adjust-stats-provide branch from da10f2e to 05d0bb3 Compare April 10, 2025 08:06

guillaumemichel and others added 3 commits April 16, 2025 10:20

update boxo dep

4e8d223

Merge branch 'master' into adjust-stats-provide

0e37989

update boxo dep

62d51d6

guillaumemichel force-pushed the adjust-stats-provide branch from 401a9e2 to 62d51d6 Compare April 16, 2025 08:46

provider worker count config

b33c4fa

guillaumemichel changed the title ~~chore: adjust provide stats~~ config: ProvideWorkerCount option Apr 16, 2025

adjust config logic

b0ba27c

guillaumemichel changed the title ~~config: ProvideWorkerCount option~~ feat(config): ProvideWorkerCount option Apr 16, 2025

guillaumemichel added 2 commits April 24, 2025 10:11

bump boxo

0662522

docs

cc02022

guillaumemichel linked an issue Apr 24, 2025 that may be closed by this pull request

Provider performance regression #10777

Closed

3 tasks

guillaumemichel marked this pull request as ready for review April 24, 2025 13:39

guillaumemichel requested a review from a team as a code owner April 24, 2025 13:39

lidel mentioned this pull request Apr 24, 2025

Release 0.35 #10760

Closed

45 tasks

lidel requested changes Apr 27, 2025

View reviewed changes

address review

5d148b8

guillaumemichel force-pushed the adjust-stats-provide branch from b9567ae to 17e0c68 Compare April 29, 2025 11:56

fixing tests

617d5c8

guillaumemichel force-pushed the adjust-stats-provide branch from 17e0c68 to 617d5c8 Compare April 29, 2025 12:05

adapt sharness tests

c65608c

guillaumemichel force-pushed the adjust-stats-provide branch from 44f8b21 to c65608c Compare April 29, 2025 13:23

guillaumemichel mentioned this pull request Apr 30, 2025

feat(config): ability to disable Bitswap fully or just server #10793

Closed

lidel added 3 commits April 30, 2025 14:33

fix(config): skip unset Provide.Strategy

30bdc15

we dont do that for Reprovider.Strategy which is more important, this should reduce user confusion

docs/chore: mark stat reprovide as experimental

9a04312

docs: Provider.Strategy

06e6618

explicitly document it is not used - without this legacy users will have it in their config and be very confused

lidel force-pushed the adjust-stats-provide branch from db5ffe9 to 06e6618 Compare April 30, 2025 13:06

lidel added 2 commits April 30, 2025 15:15

Merge 'origin/master' into adjust-stats-provide

b0f3bbd

chore: fix typo

74cf6af

lidel approved these changes Apr 30, 2025

View reviewed changes

lidel changed the title ~~feat(config): ProvideWorkerCount option~~ feat(config): Provider.WorkerCount and stats reprovider cmd Apr 30, 2025

gammazero approved these changes Apr 30, 2025

View reviewed changes

lidel changed the title ~~feat(config): Provider.WorkerCount and stats reprovider cmd~~ feat(config): Provider.WorkerCount and stats reprovide cmd Apr 30, 2025

lidel merged commit a599737 into master Apr 30, 2025
18 checks passed

lidel deleted the adjust-stats-provide branch April 30, 2025 13:32

lidel changed the title ~~feat(config): Provider.WorkerCount and stats reprovide cmd~~ feat: Provider.WorkerCount and stats reprovide cmd Apr 30, 2025

BrewTestBot mentioned this pull request May 21, 2025

ipfs 0.35.0 Homebrew/homebrew-core#224309

Merged

lidel mentioned this pull request Jun 12, 2025

Improve the providing subsystem in Kubo — IPFS/2025 ipshipyard/roadmaps#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: `Provider.WorkerCount` and `stats reprovide` cmd #10779

feat: `Provider.WorkerCount` and `stats reprovide` cmd #10779

Uh oh!

guillaumemichel commented Apr 9, 2025 •

edited by lidel

Loading

Uh oh!

guillaumemichel commented Apr 16, 2025

Uh oh!

lidel left a comment

Uh oh!

Uh oh!

Uh oh!

lidel Apr 27, 2025

Uh oh!

guillaumemichel Apr 29, 2025

Uh oh!

Uh oh!

Uh oh!

lidel left a comment

Uh oh!

gammazero Apr 30, 2025

Uh oh!

lidel Apr 30, 2025 •

edited

Loading

Uh oh!

gammazero Apr 30, 2025

Uh oh!

lidel Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

		fmt.Fprintf(wtr, "TotalProvides:\t%s\n", humanNumber(s.TotalReprovides))
		fmt.Fprintf(wtr, "AvgProvideDuration:\t%s\n", humanDuration(s.AvgReprovideDuration))


		### `Provider.Strategy`

		Legacy, not used at the moment, see [`Reprovider.Strategy`](#reproviderstrategy) instead.

	Legacy, not used at the moment, see [`Reprovider.Strategy`](#reproviderstrategy) instead.
	Legacy, no longer used, see [`Reprovider.Strategy`](#reproviderstrategy) instead.

Uh oh!

feat: Provider.WorkerCount and stats reprovide cmd #10779

feat: Provider.WorkerCount and stats reprovide cmd #10779

Uh oh!

Conversation

guillaumemichel commented Apr 9, 2025 • edited by lidel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guillaumemichel commented Apr 16, 2025

Uh oh!

lidel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lidel Apr 27, 2025

Choose a reason for hiding this comment

Uh oh!

guillaumemichel Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lidel left a comment

Choose a reason for hiding this comment

Uh oh!

gammazero Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

lidel Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gammazero Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

lidel Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

feat: `Provider.WorkerCount` and `stats reprovide` cmd #10779

feat: `Provider.WorkerCount` and `stats reprovide` cmd #10779

guillaumemichel commented Apr 9, 2025 •

edited by lidel

Loading

lidel Apr 30, 2025 •

edited

Loading