From 8820f1298b7e0341ff6b5346c6f3aadbd9598766 Mon Sep 17 00:00:00 2001 From: gammazero <11790789+gammazero@users.noreply.github.com> Date: Thu, 16 Jan 2025 22:21:38 -1000 Subject: [PATCH 1/4] reduce default number of routing in-process requests Reduce the routing ProviderQueryManager default MaxInProcessRequests from 16 to 8. This prevents a situation where goroutines can be created faster than they can be completed, leading to OOM. --- bitswap/client/client.go | 4 ++-- routing/providerquerymanager/providerquerymanager.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bitswap/client/client.go b/bitswap/client/client.go index 2e83949cb..192aac153 100644 --- a/bitswap/client/client.go +++ b/bitswap/client/client.go @@ -109,7 +109,7 @@ func WithoutDuplicatedBlockStats() Option { // lookups. The bitswap default ProviderQueryManager uses these options, which // may be more conservative than the ProviderQueryManager defaults: // -// - WithMaxInProcessRequests(16) +// - WithMaxInProcessRequests(8) // - WithMaxProviders(10) // - WithMaxTimeout(10 *time.Second) // @@ -196,7 +196,7 @@ func New(parent context.Context, network bsnet.BitSwapNetwork, providerFinder Pr if bs.providerFinder != nil && bs.defaultProviderQueryManager { // network can do dialing. pqm, err := rpqm.New(network, bs.providerFinder, - rpqm.WithMaxInProcessRequests(16), + rpqm.WithMaxInProcessRequests(8), rpqm.WithMaxProviders(10), rpqm.WithMaxTimeout(10*time.Second)) if err != nil { diff --git a/routing/providerquerymanager/providerquerymanager.go b/routing/providerquerymanager/providerquerymanager.go index 592f7f814..5564fb892 100644 --- a/routing/providerquerymanager/providerquerymanager.go +++ b/routing/providerquerymanager/providerquerymanager.go @@ -20,7 +20,7 @@ import ( var log = logging.Logger("routing/provqrymgr") const ( - defaultMaxInProcessRequests = 16 + defaultMaxInProcessRequests = 8 defaultMaxProviders = 0 defaultTimeout = 10 * time.Second ) @@ -114,7 +114,7 @@ func WithMaxTimeout(timeout time.Duration) Option { // WithMaxInProcessRequests is the maximum number of requests that can be // processed in parallel. If this is 0, then the number is unlimited. Default -// is defaultMaxInProcessRequests (16). +// is defaultMaxInProcessRequests. func WithMaxInProcessRequests(count int) Option { return func(mgr *ProviderQueryManager) error { mgr.maxInProcessRequests = count From 2fc22d2a36ba85f801424b30471225e436390201 Mon Sep 17 00:00:00 2001 From: gammazero <11790789+gammazero@users.noreply.github.com> Date: Fri, 17 Jan 2025 06:26:32 -1000 Subject: [PATCH 2/4] export providerquerymanager default consts --- .../providerquerymanager.go | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/routing/providerquerymanager/providerquerymanager.go b/routing/providerquerymanager/providerquerymanager.go index 5564fb892..b37f25fa5 100644 --- a/routing/providerquerymanager/providerquerymanager.go +++ b/routing/providerquerymanager/providerquerymanager.go @@ -20,9 +20,15 @@ import ( var log = logging.Logger("routing/provqrymgr") const ( - defaultMaxInProcessRequests = 8 - defaultMaxProviders = 0 - defaultTimeout = 10 * time.Second + // DefaultMaxInProcessRequests is the default maximum number of requests + // that are processed concurrently. A value of 0 means unlimited. + DefaultMaxInProcessRequests = 8 + // DefaultMaxProviders is the default maximum number of providers that are + // looked up per find request. 0 value means unlimited. + DefaultMaxProviders = 0 + // DefaultTimeout is the limit on the amount of time to spend waiting for + // the maximum number of providers from a find request. + DefaultTimeout = 10 * time.Second ) type inProgressRequestStatus struct { @@ -112,9 +118,9 @@ func WithMaxTimeout(timeout time.Duration) Option { } } -// WithMaxInProcessRequests is the maximum number of requests that can be -// processed in parallel. If this is 0, then the number is unlimited. Default -// is defaultMaxInProcessRequests. +// WithMaxInProcessRequests sets maximum number of requests that are processed +// concurrently. A value of 0 means unlimited. Default is +// DefaultMaxInProcessRequests. func WithMaxInProcessRequests(count int) Option { return func(mgr *ProviderQueryManager) error { mgr.maxInProcessRequests = count @@ -122,9 +128,9 @@ func WithMaxInProcessRequests(count int) Option { } } -// WithMaxProviders is the maximum number of providers that will be looked up -// per query. We only return providers that we can connect to. Defaults to 0, -// which means unbounded. +// WithMaxProviders sets the maximum number of providers that are looked up per +// find request. Only providers that we can connect to are returned. Defaults +// to 0, which means unlimited. func WithMaxProviders(count int) Option { return func(mgr *ProviderQueryManager) error { mgr.maxProviders = count @@ -140,9 +146,9 @@ func New(dialer ProviderQueryDialer, router ProviderQueryRouter, opts ...Option) dialer: dialer, router: router, providerQueryMessages: make(chan providerQueryMessage), - findProviderTimeout: defaultTimeout, - maxInProcessRequests: defaultMaxInProcessRequests, - maxProviders: defaultMaxProviders, + findProviderTimeout: DefaultTimeout, + maxInProcessRequests: DefaultMaxInProcessRequests, + maxProviders: DefaultMaxProviders, } for _, o := range opts { From ae840a23206d96fee15873118aece724fce41cfe Mon Sep 17 00:00:00 2001 From: gammazero <11790789+gammazero@users.noreply.github.com> Date: Fri, 17 Jan 2025 06:34:05 -1000 Subject: [PATCH 3/4] Only set providerquerymanager options if they differ from default --- bitswap/client/client.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bitswap/client/client.go b/bitswap/client/client.go index 192aac153..ac3f551fc 100644 --- a/bitswap/client/client.go +++ b/bitswap/client/client.go @@ -195,10 +195,7 @@ func New(parent context.Context, network bsnet.BitSwapNetwork, providerFinder Pr if bs.providerFinder != nil && bs.defaultProviderQueryManager { // network can do dialing. - pqm, err := rpqm.New(network, bs.providerFinder, - rpqm.WithMaxInProcessRequests(8), - rpqm.WithMaxProviders(10), - rpqm.WithMaxTimeout(10*time.Second)) + pqm, err := rpqm.New(network, bs.providerFinder, rpqm.WithMaxProviders(10)) if err != nil { // Should not be possible to hit this panic(err) From a2d9527fa37034765f0391ad57cb8e4e0b53c820 Mon Sep 17 00:00:00 2001 From: gammazero <11790789+gammazero@users.noreply.github.com> Date: Fri, 17 Jan 2025 06:43:58 -1000 Subject: [PATCH 4/4] Get default bitswap client settings from defaults.go --- bitswap/client/client.go | 7 +++---- bitswap/internal/defaults/defaults.go | 3 +++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/bitswap/client/client.go b/bitswap/client/client.go index ac3f551fc..f1a3cf057 100644 --- a/bitswap/client/client.go +++ b/bitswap/client/client.go @@ -109,9 +109,7 @@ func WithoutDuplicatedBlockStats() Option { // lookups. The bitswap default ProviderQueryManager uses these options, which // may be more conservative than the ProviderQueryManager defaults: // -// - WithMaxInProcessRequests(8) -// - WithMaxProviders(10) -// - WithMaxTimeout(10 *time.Second) +// - WithMaxProviders(defaults.BitswapClientDefaultMaxProviders) // // To use a custom ProviderQueryManager, set to false and wrap directly the // content router provided with the WithContentRouting() option. Only takes @@ -195,7 +193,8 @@ func New(parent context.Context, network bsnet.BitSwapNetwork, providerFinder Pr if bs.providerFinder != nil && bs.defaultProviderQueryManager { // network can do dialing. - pqm, err := rpqm.New(network, bs.providerFinder, rpqm.WithMaxProviders(10)) + pqm, err := rpqm.New(network, bs.providerFinder, + rpqm.WithMaxProviders(defaults.BitswapClientDefaultMaxProviders)) if err != nil { // Should not be possible to hit this panic(err) diff --git a/bitswap/internal/defaults/defaults.go b/bitswap/internal/defaults/defaults.go index 646b56b0d..2c5cc3668 100644 --- a/bitswap/internal/defaults/defaults.go +++ b/bitswap/internal/defaults/defaults.go @@ -10,6 +10,9 @@ const ( // broadcasting outstanding wants for the first time. ProvSearchDelay = time.Second + // Maximum number of providers that are looked up per find request by the + // default bitswap client. 0 value means unlimited. + BitswapClientDefaultMaxProviders = 10 // Number of concurrent workers in decision engine that process requests to the blockstore BitswapEngineBlockstoreWorkerCount = 128 // the total number of simultaneous threads sending outgoing messages