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

Allow to broadcast network messages in parallel #7542

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

vstakhov
Copy link
Contributor

@vstakhov vstakhov commented Jul 24, 2023

This PR addresses multiple issues pending:

  • Update orchestra to the recent version and test how the node performs
  • Add some useful metrics for outbound network bridge
  • Try to send incoming network requests to all subsystems without blocking on some particular subsystem in that loop
  • Fix all incompatibilities between orchestra and polkadot code (e.g. malus node)

This PR needs Versi burning as it involves some fundamental node components change.

@vstakhov vstakhov added I10-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. B0-silent Changes should not be mentioned in any release notes T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance. A1-needs_burnin Pull request needs to be tested on a live validator node before merge. DevOps is notified via matrix labels Jul 24, 2023
@command-bot
Copy link

command-bot bot commented Jul 24, 2023

@paritytech-cicd-pr Requester could not be detected as a member of an allowed organization.

}

let _: Vec<()> = delayed_messages.collect().await;
Copy link
Contributor

Choose a reason for hiding this comment

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

technically the bridge is still stopped until all futures complete here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but other subsystems have a chance to receive updates if some other subsystem is slow. That's the goal of this PR technically.

@vstakhov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Error: Command 'Command { std: cd "/storage/repositories/polkadot" && "git" "merge" "origin/master" "--no-ff" "--no-edit", kill_on_drop: false }' failed with status Some(1); output: no output

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/3265446

@vstakhov vstakhov marked this pull request as ready for review July 27, 2023 13:46
@vstakhov vstakhov requested a review from sandreim July 27, 2023 13:51
@vstakhov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@vstakhov
Copy link
Contributor Author

vstakhov commented Aug 1, 2023

Versi burning has not revealed any significant issues. However, we might want to burn it for a longer time with extreme disputes load and compare the metrics before/after. What do you think @sandreim ?

sender.send_messages(event.focus().map(BitfieldDistributionMessage::from)).await;
sender.send_messages(event.focus().map(ApprovalDistributionMessage::from)).await;
sender.send_messages(event.focus().map(GossipSupportMessage::from)).await;
if let Ok(msg) = event.focus().map(StatementDistributionMessage::from) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it ok to ignore a possible error?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A1-needs_burnin Pull request needs to be tested on a live validator node before merge. DevOps is notified via matrix B0-silent Changes should not be mentioned in any release notes I10-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants