From de52bc8101ad80f0dcc414547d05470d4e1f7aeb Mon Sep 17 00:00:00 2001
From: Andrew Gillis <11790789+gammazero@users.noreply.github.com>
Date: Mon, 20 Jan 2025 05:50:58 -1000
Subject: [PATCH] reduce default number of routing in-process requests (#793)

* 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.

* export providerquerymanager default consts
* Only set providerquerymanager options if they differ from default
* Get default bitswap client settings from defaults.go
---
 bitswap/client/client.go                      |  8 ++---
 bitswap/internal/defaults/defaults.go         |  3 ++
 .../providerquerymanager.go                   | 30 +++++++++++--------
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/bitswap/client/client.go b/bitswap/client/client.go
index 2e83949cb..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(16)
-//   - 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
@@ -196,9 +194,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.WithMaxProviders(10),
-			rpqm.WithMaxTimeout(10*time.Second))
+			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
diff --git a/routing/providerquerymanager/providerquerymanager.go b/routing/providerquerymanager/providerquerymanager.go
index 592f7f814..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 = 16
-	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 (16).
+// 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 {