Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Reduce ReportPeer message count in NetworkBridge #5257

Closed
sandreim opened this issue Apr 5, 2022 · 6 comments
Closed

Reduce ReportPeer message count in NetworkBridge #5257

sandreim opened this issue Apr 5, 2022 · 6 comments
Assignees
Labels
I10-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task.

Comments

@sandreim
Copy link
Contributor

sandreim commented Apr 5, 2022

Currently, a good part of the messages flowing into the network bridge subsystem are reputation changes. These messages are sent from statement-distribution, approval-distribution, collator-protocol and bitfield-distribution subsystems. To reduce the overall volume of messages that we process internally we can delay sending of the peer reputation changes so that we can aggregate multiple changes for the same target peer and send only once.

Reputation changes can be aggregated at subsystem level or in the network bridge. We should focus on the first.

One solution is to implement a peer reporting task which receives all subsystem rep changes and keeps them for a configurable interval before sending them over to substrate networking. In statement-distribution for example we need to change report_peer such that it works with the reporting task sender -

. Ideally, this is a reusable piece of code such that we can easily integrate it in all subsystems.

@sandreim sandreim added the I10-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. label Apr 5, 2022
@sandreim
Copy link
Contributor Author

sandreim commented Apr 6, 2022

I'm getting an unreasonable amount of messages around 250 messages in my tests. I'll try to understand if there is a bug causing the we high numbers or actually this is the desired behavior

@sandreim
Copy link
Contributor Author

sandreim commented Apr 6, 2022

2022-04-06 13:22:26.487 DEBUG tokio-runtime-worker parachain::peer-reporting-task: Flushing rep changes received_report_count=462 flush_count=19
2022-04-06 13:22:26.488 DEBUG tokio-runtime-worker parachain::peer-reporting-task: Report reason distribution reason="Peer was the first to provide a valid statement" count=25
2022-04-06 13:22:26.488 DEBUG tokio-runtime-worker parachain::peer-reporting-task: Report reason distribution reason="Peer provided a valid statement" count=437
2022-04-06 13:22:29.488 DEBUG tokio-runtime-worker parachain::peer-reporting-task: Flushing rep changes received_report_count=74 flush_count=19
2022-04-06 13:22:29.488 DEBUG tokio-runtime-worker parachain::peer-reporting-task: Report reason distribution reason="Peer was the first to provide a valid statement" count=3
2022-04-06 13:22:29.488 DEBUG tokio-runtime-worker parachain::peer-reporting-task: Report reason distribution reason="Peer provided a valid statement" count=71

@sandreim
Copy link
Contributor Author

sandreim commented Apr 8, 2022

#5236 contains a change that disables these messages completely. Tested that and observed that there isn't any improvement in CPU usage above the noise threshold, but based on the code analysis we can shave off CPU cycles by aggregating these messages. But.... later. Right now there are other impactful areas that should be improved.

@rphmeier
Copy link
Contributor

@sandreim So the conclusion is that the volume of peer reports to the network bridge actually isn't materially affecting performance?

@sandreim
Copy link
Contributor Author

sandreim commented Apr 25, 2022

@rphmeier Yes and no. Certainly this change improves network bridge ToF, but doesn't help with block time issues. I would suggest not merging these as we don't get anything useful for the extra complexity. WDYT?

@sandreim
Copy link
Contributor Author

fixed by #7214

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I10-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task.
Projects
None yet
Development

No branches or pull requests

2 participants