Skip to content
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

Add ConnectionTopic in order to feed the peers shard mapper with the info that HeartbeatV2 does not have #3927

Merged
merged 9 commits into from
Mar 24, 2022

Conversation

sstanculeanu
Copy link
Contributor

@sstanculeanu sstanculeanu commented Mar 21, 2022

  • added directConnectionsProcessor which sends a message with shard id to all connected peers on a separate goroutine. Added this component as a heartbeatV2Component.
  • added interceptor for ConnectionTopic used by the above component, which saves the pair pid-shard id into peer shard mapper.
  • removed crossShardStatusProcessor as it is not used anymore.
  • now networkMessenger does not implement connectionWatcher. Not needed with the new flow. Also removed currentPayloadProvider.
  • removed all the todos left for debug only and updated the network sharding test to not check for unknown peers

@sstanculeanu sstanculeanu changed the title Connection topic interceptor Refactor communication on connection topic Mar 23, 2022
removed assert on unknown peers
fixed data races
@sstanculeanu sstanculeanu marked this pull request as ready for review March 23, 2022 11:57
@iulianpascalau iulianpascalau self-requested a review March 23, 2022 12:55
cmd/node/config/config.toml Outdated Show resolved Hide resolved
heartbeat/processor/connectionsProcessor.go Outdated Show resolved Hide resolved
heartbeat/processor/connectionsProcessor_test.go Outdated Show resolved Hide resolved
integrationTests/testHeartbeatNode.go Show resolved Hide resolved
process/p2p/InterceptedShardValidatorInfo.go Outdated Show resolved Hide resolved
@ssd04 ssd04 self-requested a review March 24, 2022 06:18
@@ -690,27 +690,27 @@ func (bicf *baseInterceptorsContainerFactory) generateHeartbeatInterceptor() err

// ------- ShardValidatorInfo interceptor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed this one :D

@@ -118,7 +118,7 @@ func Test_shardValidatorInfoInterceptorProcessor_Save(t *testing.T) {
})
}

func Test_shardValidatorInfoInterceptorProcessor_DisabledMethods(t *testing.T) {
func Test_validatorInfoInterceptorProcessor_DisabledMethods(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestValidatorInfoInterceptorProcessor_DisabledMethod ?
This suggestion will comply with the rest of the project's naming conventions

assert.Nil(t, err)
assert.False(t, check.IfNil(processor))
})
}

func Test_shardValidatorInfoInterceptorProcessor_Save(t *testing.T) {
func Test_validatorInfoInterceptorProcessor_Save(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestValidatorInfoInterceptorProcessor_Save ?

@sstanculeanu sstanculeanu self-assigned this Mar 24, 2022
Copy link
Contributor

@iulianpascalau iulianpascalau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe remove the shard part from the filename:
process/p2p/interceptedShardValidatorInfo.go

@sstanculeanu sstanculeanu merged commit 4fc1c4e into feat/heartbeat-v2 Mar 24, 2022
@sstanculeanu sstanculeanu deleted the connection-interceptor branch March 24, 2022 14:37
@sstanculeanu sstanculeanu changed the title Refactor communication on connection topic Add ConnectionTopic in order to feed the peers shard mapper with the info that HeartbeatV2 does not have Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants