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

consensus: prioritize finality work over block import in queue #7307

Merged
merged 2 commits into from
Oct 16, 2020

Conversation

andresilva
Copy link
Contributor

The import queue should prioritize finality work over block import since finality work is less common and lighter than block import (especially during major sync), and performing the finality work can then lead to faster block import (or rather it solves slowdowns caused by longer stretches of unfinalized blocks).

@andresilva andresilva added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Oct 12, 2020
@andresilva andresilva force-pushed the andre/prioritize-finality-import-queue branch from 50b0580 to 2f57ae8 Compare October 12, 2020 18:08
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

A test would be really really cool :P

@@ -46,7 +48,8 @@ pub struct BasicQueue<B: BlockT, Transaction> {
impl<B: BlockT, Transaction> Drop for BasicQueue<B, Transaction> {
fn drop(&mut self) {
// Flush the queue and close the receiver to terminate the future.
self.sender.close_channel();
self.finality_sender.close_channel();
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this was done explicitly in case there's more than one instance of the sender. I don't really have a strong opinion on this.

use worker_messages::*;

let (finality_sender, mut finality_port) =
tracing_unbounded("mpsc_import_queue_worker_finality");
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this and below spsc? xD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The channel is mpsc but yeah in practice there's only one producer. But all tracing_unbounded metrics have the mpsc prefix 🤷.

@bkchr bkchr merged commit 375adf8 into master Oct 16, 2020
@bkchr bkchr deleted the andre/prioritize-finality-import-queue branch October 16, 2020 14:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants