From f8419fbb81c18bf67910c26491472f1aad46ae43 Mon Sep 17 00:00:00 2001 From: gammazero <11790789+gammazero@users.noreply.github.com> Date: Wed, 4 Dec 2024 11:11:33 -1000 Subject: [PATCH] removed Startup function from ProviderQueryManager The `providerquerymanager.New` creates a `ProvicerQueryManager` that is already started. There is no use case for starting PQM at a later time than it is created. Removing the need to call a `Statup` function separately from `New` is less convenient and can be a problem if this step is missed or if called multiple times. --- CHANGELOG.md | 1 + bitswap/client/bitswap_with_sessions_test.go | 1 - bitswap/client/client.go | 1 - routing/providerquerymanager/providerquerymanager.go | 7 ++----- .../providerquerymanager/providerquerymanager_test.go | 11 ----------- 5 files changed, 3 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4244b4b9..ab73f5cc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ The following emojis are used to highlight certain changes: - `routing/http/server`: exposes Prometheus metrics on `prometheus.DefaultRegisterer` and a custom one can be provided via `WithPrometheusRegistry` [#722](https://github.com/ipfs/boxo/pull/722) - `gateway`: `NewCacheBlockStore` and `NewCarBackend` will use `prometheus.DefaultRegisterer` when a custom one is not specified via `WithPrometheusRegistry` [#722](https://github.com/ipfs/boxo/pull/722) - `filestore`: added opt-in `WithMMapReader` option to `FileManager` to enable memory-mapped file reads [#665](https://github.com/ipfs/boxo/pull/665) +- `bitswap/routing` `ProviderQueryManager` does not require calling `Startup` separate from `New`. ### Changed diff --git a/bitswap/client/bitswap_with_sessions_test.go b/bitswap/client/bitswap_with_sessions_test.go index 110c43f5c..5d5ac8226 100644 --- a/bitswap/client/bitswap_with_sessions_test.go +++ b/bitswap/client/bitswap_with_sessions_test.go @@ -134,7 +134,6 @@ func TestCustomProviderQueryManager(t *testing.T) { if err != nil { t.Fatal(err) } - pqm.Startup() bs := bitswap.New(ctx, a.Adapter, pqm, a.Blockstore, bitswap.WithClientOption(client.WithDefaultProviderQueryManager(false))) a.Exchange.Close() // close old to be sure. diff --git a/bitswap/client/client.go b/bitswap/client/client.go index 4523962c1..5f950588a 100644 --- a/bitswap/client/client.go +++ b/bitswap/client/client.go @@ -190,7 +190,6 @@ func New(parent context.Context, network bsnet.BitSwapNetwork, providerFinder Pr // Should not be possible to hit this panic(err) } - pqm.Startup() bs.pqm = pqm } diff --git a/routing/providerquerymanager/providerquerymanager.go b/routing/providerquerymanager/providerquerymanager.go index b5a5b3fa0..98497ee66 100644 --- a/routing/providerquerymanager/providerquerymanager.go +++ b/routing/providerquerymanager/providerquerymanager.go @@ -150,12 +150,9 @@ func New(ctx context.Context, dialer ProviderQueryDialer, router ProviderQueryRo } } - return pqm, nil -} - -// Startup starts processing for the ProviderQueryManager. -func (pqm *ProviderQueryManager) Startup() { go pqm.run() + + return pqm, nil } type inProgressRequest struct { diff --git a/routing/providerquerymanager/providerquerymanager_test.go b/routing/providerquerymanager/providerquerymanager_test.go index 7369231de..1be26c4e3 100644 --- a/routing/providerquerymanager/providerquerymanager_test.go +++ b/routing/providerquerymanager/providerquerymanager_test.go @@ -76,7 +76,6 @@ func TestNormalSimultaneousFetch(t *testing.T) { } ctx := context.Background() providerQueryManager := mustNotErr(New(ctx, fpd, fpn)) - providerQueryManager.Startup() keys := random.Cids(2) sessionCtx, cancel := context.WithTimeout(ctx, 5*time.Second) @@ -114,7 +113,6 @@ func TestDedupingProviderRequests(t *testing.T) { } ctx := context.Background() providerQueryManager := mustNotErr(New(ctx, fpd, fpn)) - providerQueryManager.Startup() key := random.Cids(1)[0] sessionCtx, cancel := context.WithTimeout(ctx, 5*time.Second) @@ -155,7 +153,6 @@ func TestCancelOneRequestDoesNotTerminateAnother(t *testing.T) { } ctx := context.Background() providerQueryManager := mustNotErr(New(ctx, fpd, fpn)) - providerQueryManager.Startup() key := random.Cids(1)[0] @@ -202,7 +199,6 @@ func TestCancelManagerExitsGracefully(t *testing.T) { managerCtx, managerCancel := context.WithTimeout(ctx, 5*time.Millisecond) defer managerCancel() providerQueryManager := mustNotErr(New(managerCtx, fpd, fpn)) - providerQueryManager.Startup() key := random.Cids(1)[0] @@ -238,7 +234,6 @@ func TestPeersWithConnectionErrorsNotAddedToPeerList(t *testing.T) { } ctx := context.Background() providerQueryManager := mustNotErr(New(ctx, fpd, fpn)) - providerQueryManager.Startup() key := random.Cids(1)[0] @@ -275,7 +270,6 @@ func TestRateLimitingRequests(t *testing.T) { ctx, cancel := context.WithCancel(ctx) defer cancel() providerQueryManager := mustNotErr(New(ctx, fpd, fpn, WithMaxInProcessRequests(maxInProcessRequests))) - providerQueryManager.Startup() keys := random.Cids(maxInProcessRequests + 1) sessionCtx, cancel := context.WithTimeout(ctx, 5*time.Second) @@ -317,7 +311,6 @@ func TestUnlimitedRequests(t *testing.T) { ctx, cancel := context.WithCancel(ctx) defer cancel() providerQueryManager := mustNotErr(New(ctx, fpd, fpn, WithMaxInProcessRequests(0))) - providerQueryManager.Startup() keys := random.Cids(inProcessRequests) sessionCtx, cancel := context.WithTimeout(ctx, 5*time.Second) @@ -355,7 +348,6 @@ func TestFindProviderTimeout(t *testing.T) { } ctx := context.Background() providerQueryManager := mustNotErr(New(ctx, fpd, fpn, WithMaxTimeout(2*time.Millisecond))) - providerQueryManager.Startup() keys := random.Cids(1) sessionCtx, cancel := context.WithTimeout(ctx, 5*time.Second) @@ -379,7 +371,6 @@ func TestFindProviderPreCanceled(t *testing.T) { } ctx := context.Background() providerQueryManager := mustNotErr(New(ctx, fpd, fpn, WithMaxTimeout(100*time.Millisecond))) - providerQueryManager.Startup() keys := random.Cids(1) sessionCtx, cancel := context.WithCancel(ctx) @@ -404,7 +395,6 @@ func TestCancelFindProvidersAfterCompletion(t *testing.T) { } ctx := context.Background() providerQueryManager := mustNotErr(New(ctx, fpd, fpn, WithMaxTimeout(100*time.Millisecond))) - providerQueryManager.Startup() keys := random.Cids(1) sessionCtx, cancel := context.WithCancel(ctx) @@ -437,7 +427,6 @@ func TestLimitedProviders(t *testing.T) { } ctx := context.Background() providerQueryManager := mustNotErr(New(ctx, fpd, fpn, WithMaxProviders(max), WithMaxTimeout(100*time.Millisecond))) - providerQueryManager.Startup() keys := random.Cids(1) providersChan := providerQueryManager.FindProvidersAsync(ctx, keys[0], 0)