-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Full archive peer shard mapper #5337
Full archive peer shard mapper #5337
Conversation
small refactor on antiflood components creation created a new full history peer shard mapper create new interceptors for heartbeat to feed the new peer shard mapper
…or_fullarchive_messenger
node/nodeHelper.go
Outdated
peerDenialEvaluator, err := preparePeerDenialEvaluators(networkComponents, processComponents) | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we don't need to return full achive peer denial evaluator and set it when creating NewNode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope.. the peer denial evaluator at node level is used only to check if a peer is denied, which relays on 2 shared components for both regular denial evaluator and full archive one.. so the response would be the same
return bicf.container.Add(identifierHeartbeat, interceptor) | ||
} | ||
|
||
func (bicf *baseInterceptorsContainerFactory) createOneHeartbeatV2Interceptor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func (bicf *baseInterceptorsContainerFactory) createOneHeartbeatV2Interceptor( | |
func (bicf *baseInterceptorsContainerFactory) createHeartbeatV2Interceptor( |
?
return bicf.container.Add(identifier, interceptor) | ||
} | ||
|
||
func (bicf *baseInterceptorsContainerFactory) createOnePeerShardInterceptor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func (bicf *baseInterceptorsContainerFactory) createOnePeerShardInterceptor( | |
func (bicf *baseInterceptorsContainerFactory) createPeerShardInterceptor( |
?
@@ -53,7 +53,8 @@ func createTestProcessorNodeAndTrieStorage( | |||
TrieStore: mainStorer, | |||
GasScheduleMap: createTestGasMap(), | |||
}) | |||
_ = node.Messenger.CreateTopic(common.ConsensusTopic+node.ShardCoordinator.CommunicationIdentifier(node.ShardCoordinator.SelfId()), true) | |||
_ = node.MainMessenger.CreateTopic(common.ConsensusTopic+node.ShardCoordinator.CommunicationIdentifier(node.ShardCoordinator.SelfId()), true) | |||
_ = node.FullArchiveMessenger.CreateTopic(common.ConsensusTopic+node.ShardCoordinator.CommunicationIdentifier(node.ShardCoordinator.SelfId()), true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not required
If this doesn't panic, do we need to close all messengers on the defer functions? (valid on all occurrences)
Maybe create a function on the test processor node that will close all messengers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the test processor node already has a Close() method that closes all messengers.. updated tests to call it
process/factory/interceptorscontainer/baseInterceptorsContainerFactory.go
Outdated
Show resolved
Hide resolved
@@ -254,7 +254,7 @@ func testNodeRequestInterceptTrieNodesWithMessengerNotSyncingShouldErr(t *testin | |||
go func() { | |||
// sudden close of the resolver node after just 2 seconds | |||
time.Sleep(time.Second * 2) | |||
_ = nResolver.Messenger.Close() | |||
_ = nResolver.MainMessenger.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_ = nResolver.MainMessenger.Close() | |
nResolver.Close() |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update all tests
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## feat/multiple_p2p_messengers #5337 +/- ##
================================================================
+ Coverage 80.26% 80.34% +0.07%
================================================================
Files 695 695
Lines 90244 90370 +126
================================================================
+ Hits 72432 72604 +172
+ Misses 12631 12599 -32
+ Partials 5181 5167 -14
☔ View full report in Codecov by Sentry. |
} | ||
|
||
return container, nil | ||
if args.NodeOperationMode == p2p.FullArchiveMode { | ||
err = interceptorsContainerFactory.AddShardTrieNodeInterceptors(fullArchiveContainer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be left as it is but we do not support trie sync in an arbitrary epoch. Debatable if we will support at a certain time.
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
feat
branch created?feat
branch merging, do all satellite projects have a proper tag insidego.mod
?