[Networking] Optimizing GossipSub RPC Handling Memory Usage Through Asynchronous Subscription Updates #4988

yhassanzadeh13 · 2023-11-09T23:23:14Z

Problem Statement:
Our current GossipSub implementation encounters significant memory overhead due to the frequent updates triggered by the peer-scoring component. This component, integral for verifying peer subscriptions, becomes increasingly memory-intensive with rising RPC traffic. Each update renews the router’s topic list and peers' roster, and while individually lightweight, these updates cumulatively strain the system. This issue was highlighted in a flame graph analysis, pointing to a need for optimization.

Read more here: https://github.com/dapperlabs/flow-go/issues/6870

Proposed Solution:
This PR introduces a major optimization to the subscription update mechanism within the GossipSub router. We propose shifting from an event-based update system to a time-based approach. Specifically, this involves:

Implementing an asynchronous logic to update subscriptions at predefined, longer intervals (e.g., once every 10 minutes).
Introducing a configurable update interval, allowing system administrators to adjust the frequency according to network needs.
Integrating HeroCache in place of sync.Map for monitoring subscriptions, aiming to reduce heap allocations and improve overall performance.

Key Features:

Asynchronous Update Logic: Regular, long-interval updates rather than event-based updates to minimize the frequency of memory-intensive operations.
Configurable Update Intervals: Customizable intervals for subscription updates, adjustable via a new flag.
HeroCache Integration: Leveraging HeroCache for more efficient subscription monitoring, optimizing memory usage.

This reverts commit 086fdf8.

This reverts commit 02b1d57.

…ry-intensive-issues-part-1

This reverts commit 9b1a4cc.

…ry-intensive-issues-part-1

kc1116 · 2023-11-15T19:37:53Z

network/p2p/p2pconf/gossipsub.go

+	SubscriptionProviderConfig SubscriptionProviderParameters `mapstructure:",squash"`
+}
+
+type SubscriptionProviderParameters struct {


Are there concrete minimum values we can use for the validate tags of this struct? For SubscriptionUpdateInterval perhaps we can use 10m as minimum, and CacheSize the current amount of nodes on the current network.

Thanks for your input. Setting a fixed minimum seems risky. Presently, validation mandates parameters to be greater than zero, ensuring they're actively configured. Allowing node operators to adjust parameters to minimal values as necessary seems more prudent.

kc1116 · 2023-11-15T20:15:35Z

network/p2p/scoring/internal/subscriptionCache.go

+
+type SubscriptionRecordCache struct {
+	c            *stdmap.Backend
+	currentCycle atomic.Uint64


Can you add some documentation describing the relationship between the currentCycle and how it is moved during each updateTopics time interval which is configured. It's sort of implied here but a short concise description would be good.

network/p2p/scoring/subscription_provider.go

kc1116

Left a few small comments otherwise looks great !

peterargue

looks good. added a few small comments

peterargue · 2023-11-15T23:05:43Z

network/p2p/scoring/registry.go

+		case <-reg.validator.Ready():
+			reg.logger.Info().Msg("subscription validator started")
+		}
+		ready()


I assume the worker isn't ready() unless the validator is

Suggested change

case <-reg.validator.Ready():

reg.logger.Info().Msg("subscription validator started")

}

ready()

case <-reg.validator.Ready():

reg.logger.Info().Msg("subscription validator started")

ready()

}

peterargue · 2023-11-15T23:06:22Z

network/p2p/scoring/score_option.go

+		case <-scoreRegistry.Ready():
+			s.logger.Info().Msg("score registry started")
+		}
+		ready()


same here

Suggested change

case <-scoreRegistry.Ready():

s.logger.Info().Msg("score registry started")

}

ready()

case <-scoreRegistry.Ready():

s.logger.Info().Msg("score registry started")

ready()

}

peterargue · 2023-11-15T23:09:35Z

network/p2p/scoring/subscription_provider.go

+			updatedTopics, err := s.cache.AddTopicForPeer(p, topic)
+			if err != nil {
+				// this is an irrecoverable error; hence, we crash the node.
+				ctx.Throw(fmt.Errorf("failed to update topics for peer %s: %w", p, err))


nit: rather than passing the irrecoverable.SignalerContext, have this return an error, and the loop can throw if an error is returned.

peterargue · 2023-11-15T23:10:51Z

network/p2p/scoring/subscription_validator.go

+		AddWorker(func(ctx irrecoverable.SignalerContext, ready component.ReadyFunc) {
+			logger.Debug().Msg("starting subscription validator")
+			v.subscriptionProvider.Start(ctx)
+			<-v.subscriptionProvider.Ready()


does this component become ready quickly? if not, it's probably worth using a select with the context to allow graceful aborts

Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>

…b.com:onflow/flow-go into yahya/6870-fix-memory-intensive-issues-part-1

yhassanzadeh13 added 28 commits October 4, 2023 10:37

repackages spam record cache

02b1d57

repackages scoring

086fdf8

Revert "repackages scoring"

7c926e9

This reverts commit 086fdf8.

Revert "repackages spam record cache"

7a4cd44

This reverts commit 02b1d57.

replaces event-based topic update with regular intervals

ba89a09

wires subscription provider parameters to gossipsub builder

9d3a0f3

wires subscription provider parameters to libp2p builder

e817929

wires down the subscription config parameters

b162ff0

wires flags and default configs for interval

ca81493

wires flags

77046a2

merges master into branch

2f4c179

lint fix

c4cd07a

fixes TestSubscriptionProvider_GetSubscribedTopics

81cc007

wip

6a7d5c2

wip

a832465

implements subscription record entity

e550b6e

wip

284506c

refines subscription record cache

22c0f89

adds component lifecycle to subscription validator, provider and option

d1a4bf9

Merge branch 'master' into yahya/6870-fix-memory-intensive-issues-part-1

801640b

fixes circular dependency between GossipSub and inspector

0c533a7

lint fix

b105b36

updates mocks

594748d

build fix

f3a3581

makes app-specific score registry startable

3af7376

adds startup hierarchy to score option

a0ff8ec

fixes all tests

e72cd25

Merge remote-tracking branch 'origin/master' into yahya/6870-fix-memo…

55cbb6e

…ry-intensive-issues-part-1

yhassanzadeh13 changed the title ~~[Networking] Fixing part-1 of RPC inspection resource intensive operations~~ [Networking] Fixing RPC inspection memory intensive operation (subscription validator) Nov 9, 2023

adds duration to the startup log

f253b09

yhassanzadeh13 and others added 8 commits November 14, 2023 13:28

Merge branch 'master' into yahya/6870-fix-memory-intensive-issues-part-1

e13a94f

consolidates PeerIdFixtureB with PeerIdFixture

443bf8a

adds lint and tidy to insecure package

08bf30e

lint fix

9b1a4cc

Revert "lint fix"

cc27bad

This reverts commit 9b1a4cc.

lint fix

99bc1fd

Merge branch 'master' into yahya/6870-fix-memory-intensive-issues-part-1

f7d1eef

Merge remote-tracking branch 'origin/master' into yahya/6870-fix-memo…

f34bc32

…ry-intensive-issues-part-1

kc1116 reviewed Nov 15, 2023

View reviewed changes

network/p2p/scoring/subscription_provider.go Outdated Show resolved Hide resolved

kc1116 approved these changes Nov 15, 2023

View reviewed changes

peterargue approved these changes Nov 15, 2023

View reviewed changes

yhassanzadeh13 and others added 11 commits November 16, 2023 13:04

moving ready to the select-case

48f255f

moving ready to select-case

e3525d3

revises the error by update loop to make it irrecoverable

a46c466

adds select case for startup of subscription provider

d83fab5

Update network/p2p/scoring/subscription_provider.go

c0d7e07

Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>

adds documentation to subscription record cache

ddfcc9f

Merge branch 'yahya/6870-fix-memory-intensive-issues-part-1' of githu…

a433ec1

…b.com:onflow/flow-go into yahya/6870-fix-memory-intensive-issues-part-1

Merge branch 'master' into yahya/6870-fix-memory-intensive-issues-part-1

ba7b727

fixes a bug with subscription record id

db47e4d

Merge branch 'yahya/6870-fix-memory-intensive-issues-part-1' of githu…

4853b4c

…b.com:onflow/flow-go into yahya/6870-fix-memory-intensive-issues-part-1

adds more documentation for current cycle

f8daccd

yhassanzadeh13 added this pull request to the merge queue Nov 16, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 16, 2023

yhassanzadeh13 added this pull request to the merge queue Nov 16, 2023

Merged via the queue into master with commit 4223549 Nov 16, 2023
54 checks passed

yhassanzadeh13 deleted the yahya/6870-fix-memory-intensive-issues-part-1 branch November 16, 2023 23:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Networking] Optimizing GossipSub RPC Handling Memory Usage Through Asynchronous Subscription Updates #4988

[Networking] Optimizing GossipSub RPC Handling Memory Usage Through Asynchronous Subscription Updates #4988

yhassanzadeh13 commented Nov 9, 2023 •

edited

Loading

kc1116 Nov 15, 2023

yhassanzadeh13 Nov 16, 2023

kc1116 Nov 15, 2023

yhassanzadeh13 Nov 16, 2023

kc1116 left a comment

peterargue left a comment

peterargue Nov 15, 2023

yhassanzadeh13 Nov 16, 2023

peterargue Nov 15, 2023

yhassanzadeh13 Nov 16, 2023 •

edited

Loading

peterargue Nov 15, 2023

yhassanzadeh13 Nov 16, 2023

peterargue Nov 15, 2023

yhassanzadeh13 Nov 16, 2023

[Networking] Optimizing GossipSub RPC Handling Memory Usage Through Asynchronous Subscription Updates #4988

[Networking] Optimizing GossipSub RPC Handling Memory Usage Through Asynchronous Subscription Updates #4988

Conversation

yhassanzadeh13 commented Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kc1116 left a comment

Choose a reason for hiding this comment

peterargue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yhassanzadeh13 Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yhassanzadeh13 commented Nov 9, 2023 •

edited

Loading

yhassanzadeh13 Nov 16, 2023 •

edited

Loading