Update request-resolve mechanism for full archive network #5347
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added separate preferred peers holder for the new network
Full archive integration test
…//github.com/multiversx/mx-chain-go into requesters_resolvers_full_archive_network # Conflicts: # factory/processing/processComponents.go # factory/processing/processComponentsHandler_test.go # integrationTests/multiShard/hardFork/hardFork_test.go # update/factory/exportHandlerFactory.go
Base automatically changed from
process_components_for_fullarchive_messenger
to
feat/multiple_p2p_messengers
June 16, 2023 09:21
…_full_archive_network
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## feat/multiple_p2p_messengers #5347 +/- ##
================================================================
- Coverage 80.34% 80.33% -0.01%
================================================================
Files 695 695
Lines 90370 90497 +127
================================================================
+ Hits 72604 72698 +94
- Misses 12599 12630 +31
- Partials 5167 5169 +2
☔ View full report in Codecov by Sentry. |
ssd04
reviewed
Jun 16, 2023
Comment on lines
23
to
26
| // Get does nothing as it is disabled | ||
| func (holder *preferredPeersHolder) Get() map[uint32][]core.PeerID { | ||
| return make(map[uint32][]core.PeerID) | ||
| } |
There was a problem hiding this comment.
Suggested change
| // Get does nothing as it is disabled | |
| func (holder *preferredPeersHolder) Get() map[uint32][]core.PeerID { | |
| return make(map[uint32][]core.PeerID) | |
| } | |
| // Get returns empty map as it is disabled | |
| func (holder *preferredPeersHolder) Get() map[uint32][]core.PeerID { | |
| return make(map[uint32][]core.PeerID) | |
| } |
?
Comment on lines
263
to
271
| func (mnc *managedNetworkComponents) FullArchivePreferredPeersHolderHandler() factory.PreferredPeersHolderHandler { | ||
| mnc.mutNetworkComponents.RLock() | ||
| defer mnc.mutNetworkComponents.RUnlock() | ||
|
|
||
| if mnc.networkComponents == nil { | ||
| return nil | ||
| } | ||
|
|
||
| return mnc.mainNetworkHolder.preferredPeersHolder |
There was a problem hiding this comment.
Suggested change
| func (mnc *managedNetworkComponents) FullArchivePreferredPeersHolderHandler() factory.PreferredPeersHolderHandler { | |
| mnc.mutNetworkComponents.RLock() | |
| defer mnc.mutNetworkComponents.RUnlock() | |
| if mnc.networkComponents == nil { | |
| return nil | |
| } | |
| return mnc.mainNetworkHolder.preferredPeersHolder | |
| func (mnc *managedNetworkComponents) FullArchivePreferredPeersHolderHandler() factory.PreferredPeersHolderHandler { | |
| mnc.mutNetworkComponents.RLock() | |
| defer mnc.mutNetworkComponents.RUnlock() | |
| if mnc.networkComponents == nil { | |
| return nil | |
| } | |
| return mnc.fullArchiveNetworkHolder.preferredPeersHolder |
?
There was a problem hiding this comment.
updated, good catch
cmd/node/config/prefs.toml
Outdated
| # "127.0.0.10", | ||
| # "16Uiu2HAm6yvbp1oZ6zjnWsn9FdRqBSaQkbhELyaThuq48ybdorrr" | ||
| # ] | ||
| PreferredFullArchiveConnections = [] |
There was a problem hiding this comment.
I think we can use the same PreferredConnections list for all network messengers
There was a problem hiding this comment.
removed and reused the old one
| @@ -31,7 +31,11 @@ func NewTopicResolverSender(arg ArgTopicResolverSender) (*topicResolverSender, e | |||
| // Send is used to send an array buffer to a connected peer | |||
| // It is used when replying to a request | |||
| func (trs *topicResolverSender) Send(buff []byte, peer core.PeerID) error { | |||
| return trs.sendToConnectedPeer(trs.topicName, buff, peer) | |||
| if trs.fullArchiveMessenger.IsConnected(peer) { | |||
There was a problem hiding this comment.
so this is how we manage to send on the "correct" network
The other possibility was to provide the dataretriever.MessageHandler instance on this function. This variant will ensure that performance degradation will not occur.
There was a problem hiding this comment.
added TODO and will be fixed in the next PR, as discussed
| @@ -259,6 +259,18 @@ func (mnc *managedNetworkComponents) FullArchivePeersRatingMonitor() p2p.PeersRa | |||
| return mnc.fullArchiveNetworkHolder.peersRatingMonitor | |||
| } | |||
|
|
|||
| // FullArchivePreferredPeersHolderHandler returns the preferred peers holder of the full archive network | |||
| func (mnc *managedNetworkComponents) FullArchivePreferredPeersHolderHandler() factory.PreferredPeersHolderHandler { | |||
There was a problem hiding this comment.
I do not think this is required
| @@ -111,12 +113,103 @@ func TestPeersRatingAndResponsiveness(t *testing.T) { | |||
| assert.Equal(t, finalMaliciousExpectedRating, maliciousRating) | |||
| } | |||
|
|
|||
| func createNodeWithPeersRatingHandler(shardID uint32, numShards uint32) *integrationTests.TestProcessorNode { | |||
| func TestPeersRatingAndResponsivenessOnFullArchive(t *testing.T) { | |||
There was a problem hiding this comment.
👍
very good test for ensuring that the feature works
ssd04
approved these changes
Jun 20, 2023
iulianpascalau
approved these changes
Jun 21, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?