Skip to content

Commit

Permalink
slots: incrementally backoff claiming slots if finality lags behind (p…
Browse files Browse the repository at this point in the history
…aritytech#7186)

* babe: backoff authoring blocks when finality lags

* babe: move backoff authoring params to default constructor

* babe: deduplicate the test a bit

* babe: set backoff constants in service

* babe: use better names for backoff authoring block parameters

* babe: remove last unwrap

* babe: slight style tweak

* babe: fix comment

* slots: move backoff block authorship logic to SimpleSlotWorker

* aura: append SlotInfo in on_slot

* slots: use the correct types for parameters

* slots: fix review comments

* aura: add missing backoff authoring blocks parameters

* slots: add comments for default values

* slots: add additional checks in test

* slots: update implementation for new master

* slots: revert the change to SlotInfo

* Fix review comments

* slots: rework unit tests for backing off claiming slots

* slots: add test for asymptotic behaviour for slot claims

* slots: address review comments

* slots: add test for max_interval

* slots: add assertion for intervals between between claimed slots

* slots: remove rustfmt directive

* slots: another attempt at explaining authoring_rate

* slots: up unfinalized_slack to 50 by default

* slots: add tests for time to reach max_interval

* slots: fix typo in comments

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* slots: additional tweaks to comments and info calls

* slots: rename to BackoffAuthoringOnFinalizedHeadLagging

* slots: make the backing off strategy generic

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* slots: implement backoff trait for () for simplicity

* slots: move logging inside backing off function to make it more specific

* aura: add missing function parameter

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
  • Loading branch information
2 people authored and darkfriend77 committed Jan 11, 2021
1 parent 84af538 commit 1bde9ac
Show file tree
Hide file tree
Showing 9 changed files with 540 additions and 20 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion bin/node-template/node/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {

let role = config.role.clone();
let force_authoring = config.force_authoring;
let backoff_authoring_blocks: Option<()> = None;
let name = config.network.node_name.clone();
let enable_grandpa = !config.disable_grandpa;
let prometheus_registry = config.prometheus_registry().cloned();
Expand Down Expand Up @@ -155,7 +156,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
let can_author_with =
sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone());

let aura = sc_consensus_aura::start_aura::<_, _, _, _, _, AuraPair, _, _, _>(
let aura = sc_consensus_aura::start_aura::<_, _, _, _, _, AuraPair, _, _, _,_>(
sc_consensus_aura::slot_duration(&*client)?,
client.clone(),
select_chain,
Expand All @@ -164,6 +165,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
network.clone(),
inherent_data_providers.clone(),
force_authoring,
backoff_authoring_blocks,
keystore_container.sync_keystore(),
can_author_with,
)?;
Expand Down
1 change: 1 addition & 0 deletions bin/node/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ sc-chain-spec = { version = "2.0.0", path = "../../../client/chain-spec" }
sc-consensus = { version = "0.8.0", path = "../../../client/consensus/common" }
sc-transaction-pool = { version = "2.0.0", path = "../../../client/transaction-pool" }
sc-network = { version = "0.8.0", path = "../../../client/network" }
sc-consensus-slots = { version = "0.8.0", path = "../../../client/consensus/slots" }
sc-consensus-babe = { version = "0.8.0", path = "../../../client/consensus/babe" }
grandpa = { version = "0.8.0", package = "sc-finality-grandpa", path = "../../../client/finality-grandpa" }
sc-client-db = { version = "0.8.0", default-features = false, path = "../../../client/db" }
Expand Down
3 changes: 3 additions & 0 deletions bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ pub fn new_full_base(

let role = config.role.clone();
let force_authoring = config.force_authoring;
let backoff_authoring_blocks =
Some(sc_consensus_slots::BackoffAuthoringOnFinalizedHeadLagging::default());
let name = config.network.node_name.clone();
let enable_grandpa = !config.disable_grandpa;
let prometheus_registry = config.prometheus_registry().cloned();
Expand Down Expand Up @@ -249,6 +251,7 @@ pub fn new_full_base(
sync_oracle: network.clone(),
inherent_data_providers: inherent_data_providers.clone(),
force_authoring,
backoff_authoring_blocks,
babe_link,
can_author_with,
};
Expand Down
40 changes: 32 additions & 8 deletions client/consensus/aura/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use sp_core::crypto::Public;
use sp_application_crypto::{AppKey, AppPublic};
use sp_runtime::{
generic::{BlockId, OpaqueDigestItemId},
Justification,
traits::NumberFor, Justification,
};
use sp_runtime::traits::{Block as BlockT, Header, DigestItemFor, Zero, Member};
use sp_api::ProvideRuntimeApi;
Expand All @@ -73,6 +73,7 @@ use sc_telemetry::{telemetry, CONSENSUS_TRACE, CONSENSUS_DEBUG, CONSENSUS_INFO};

use sc_consensus_slots::{
CheckedHeader, SlotInfo, SlotCompatible, StorageChanges, check_equivocation,
BackoffAuthoringBlocksStrategy,
};

use sp_api::ApiExt;
Expand Down Expand Up @@ -138,7 +139,7 @@ impl SlotCompatible for AuraSlotCompatible {
}

/// Start the aura worker. The returned future should be run in a futures executor.
pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
pub fn start_aura<B, C, SC, E, I, P, SO, CAW, BS, Error>(
slot_duration: SlotDuration,
client: Arc<C>,
select_chain: SC,
Expand All @@ -147,11 +148,12 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
sync_oracle: SO,
inherent_data_providers: InherentDataProviders,
force_authoring: bool,
backoff_authoring_blocks: Option<BS>,
keystore: SyncCryptoStorePtr,
can_author_with: CAW,
) -> Result<impl Future<Output = ()>, sp_consensus::Error> where
B: BlockT,
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B> + AuxStore + Send + Sync,
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B> + AuxStore + HeaderBackend<B> + Send + Sync,
C::Api: AuraApi<B, AuthorityId<P>>,
SC: SelectChain<B>,
E: Environment<B, Error = Error> + Send + Sync + 'static,
Expand All @@ -163,6 +165,7 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
SO: SyncOracle + Send + Sync + Clone,
CAW: CanAuthorWith<B> + Send,
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + 'static,
{
let worker = AuraWorker {
client,
Expand All @@ -171,6 +174,7 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
keystore,
sync_oracle: sync_oracle.clone(),
force_authoring,
backoff_authoring_blocks,
_key_type: PhantomData::<P>,
};
register_aura_inherent_data_provider(
Expand All @@ -188,20 +192,22 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
))
}

struct AuraWorker<C, E, I, P, SO> {
struct AuraWorker<C, E, I, P, SO, BS> {
client: Arc<C>,
block_import: Arc<Mutex<I>>,
env: E,
keystore: SyncCryptoStorePtr,
sync_oracle: SO,
force_authoring: bool,
backoff_authoring_blocks: Option<BS>,
_key_type: PhantomData<P>,
}

impl<B, C, E, I, P, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for AuraWorker<C, E, I, P, SO>
impl<B, C, E, I, P, Error, SO, BS> sc_consensus_slots::SimpleSlotWorker<B>
for AuraWorker<C, E, I, P, SO, BS>
where
B: BlockT,
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B> + Sync,
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B> + HeaderBackend<B> + Sync,
C::Api: AuraApi<B, AuthorityId<P>>,
E: Environment<B, Error = Error>,
E::Proposer: Proposer<B, Error = Error, Transaction = sp_api::TransactionFor<C, B>>,
Expand All @@ -210,6 +216,7 @@ where
P::Public: AppPublic + Public + Member + Encode + Decode + Hash,
P::Signature: TryFrom<Vec<u8>> + Member + Encode + Decode + Hash + Debug,
SO: SyncOracle + Send + Clone,
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + 'static,
Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
{
type BlockImport = I;
Expand Down Expand Up @@ -316,6 +323,21 @@ where
self.force_authoring
}

fn should_backoff(&self, slot_number: u64, chain_head: &B::Header) -> bool {
if let Some(ref strategy) = self.backoff_authoring_blocks {
if let Ok(chain_head_slot) = find_pre_digest::<B, P>(chain_head) {
return strategy.should_backoff(
*chain_head.number(),
chain_head_slot,
self.client.info().finalized_number,
slot_number,
self.logging_target(),
);
}
}
false
}

fn sync_oracle(&mut self) -> &mut Self::SyncOracle {
&mut self.sync_oracle
}
Expand Down Expand Up @@ -863,7 +885,7 @@ mod tests {
use sp_keyring::sr25519::Keyring;
use sc_client_api::BlockchainEvents;
use sp_consensus_aura::sr25519::AuthorityPair;
use sc_consensus_slots::SimpleSlotWorker;
use sc_consensus_slots::{SimpleSlotWorker, BackoffAuthoringOnFinalizedHeadLagging};
use std::task::Poll;
use sc_block_builder::BlockBuilderProvider;
use sp_runtime::traits::Header as _;
Expand Down Expand Up @@ -1012,7 +1034,7 @@ mod tests {
&inherent_data_providers, slot_duration.get()
).expect("Registers aura inherent data provider");

aura_futures.push(start_aura::<_, _, _, _, _, AuthorityPair, _, _, _>(
aura_futures.push(start_aura::<_, _, _, _, _, AuthorityPair, _, _, _, _>(
slot_duration,
client.clone(),
select_chain,
Expand All @@ -1021,6 +1043,7 @@ mod tests {
DummyOracle,
inherent_data_providers,
false,
Some(BackoffAuthoringOnFinalizedHeadLagging::default()),
keystore,
sp_consensus::AlwaysCanAuthor,
).expect("Starts aura"));
Expand Down Expand Up @@ -1081,6 +1104,7 @@ mod tests {
keystore: keystore.into(),
sync_oracle: DummyOracle.clone(),
force_authoring: false,
backoff_authoring_blocks: Some(BackoffAuthoringOnFinalizedHeadLagging::default()),
_key_type: PhantomData::<AuthorityPair>,
};

Expand Down
38 changes: 33 additions & 5 deletions client/consensus/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ use log::{debug, info, log, trace, warn};
use prometheus_endpoint::Registry;
use sc_consensus_slots::{
SlotInfo, SlotCompatible, StorageChanges, CheckedHeader, check_equivocation,
BackoffAuthoringBlocksStrategy,
};
use sc_consensus_epochs::{
descendent_query, SharedEpochChanges, EpochChangesFor, Epoch as EpochT, ViableEpochDescriptor,
Expand Down Expand Up @@ -354,7 +355,7 @@ impl std::ops::Deref for Config {
}

/// Parameters for BABE.
pub struct BabeParams<B: BlockT, C, E, I, SO, SC, CAW> {
pub struct BabeParams<B: BlockT, C, E, I, SO, SC, CAW, BS> {
/// The keystore that manages the keys of the node.
pub keystore: SyncCryptoStorePtr,

Expand All @@ -381,6 +382,9 @@ pub struct BabeParams<B: BlockT, C, E, I, SO, SC, CAW> {
/// Force authoring of blocks even if we are offline
pub force_authoring: bool,

/// Strategy and parameters for backing off block production.
pub backoff_authoring_blocks: Option<BS>,

/// The source of timestamps for relative slots
pub babe_link: BabeLink<B>,

Expand All @@ -389,7 +393,7 @@ pub struct BabeParams<B: BlockT, C, E, I, SO, SC, CAW> {
}

/// Start the babe worker.
pub fn start_babe<B, C, SC, E, I, SO, CAW, Error>(BabeParams {
pub fn start_babe<B, C, SC, E, I, SO, CAW, BS, Error>(BabeParams {
keystore,
client,
select_chain,
Expand All @@ -398,9 +402,10 @@ pub fn start_babe<B, C, SC, E, I, SO, CAW, Error>(BabeParams {
sync_oracle,
inherent_data_providers,
force_authoring,
backoff_authoring_blocks,
babe_link,
can_author_with,
}: BabeParams<B, C, E, I, SO, SC, CAW>) -> Result<
}: BabeParams<B, C, E, I, SO, SC, CAW, BS>) -> Result<
BabeWorker<B>,
sp_consensus::Error,
> where
Expand All @@ -416,6 +421,7 @@ pub fn start_babe<B, C, SC, E, I, SO, CAW, Error>(BabeParams {
Error: std::error::Error + Send + From<ConsensusError> + From<I::Error> + 'static,
SO: SyncOracle + Send + Sync + Clone + 'static,
CAW: CanAuthorWith<B> + Send + 'static,
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + 'static,
{
let config = babe_link.config;
let slot_notification_sinks = Arc::new(Mutex::new(Vec::new()));
Expand All @@ -426,6 +432,7 @@ pub fn start_babe<B, C, SC, E, I, SO, CAW, Error>(BabeParams {
env,
sync_oracle: sync_oracle.clone(),
force_authoring,
backoff_authoring_blocks,
keystore,
epoch_changes: babe_link.epoch_changes.clone(),
slot_notification_sinks: slot_notification_sinks.clone(),
Expand Down Expand Up @@ -490,19 +497,22 @@ impl<B: BlockT> futures::Future for BabeWorker<B> {
/// Slot notification sinks.
type SlotNotificationSinks<B> = Arc<Mutex<Vec<Sender<(u64, ViableEpochDescriptor<<B as BlockT>::Hash, NumberFor<B>, Epoch>)>>>>;

struct BabeSlotWorker<B: BlockT, C, E, I, SO> {
struct BabeSlotWorker<B: BlockT, C, E, I, SO, BS> {
client: Arc<C>,
block_import: Arc<Mutex<I>>,
env: E,
sync_oracle: SO,
force_authoring: bool,
backoff_authoring_blocks: Option<BS>,
keystore: SyncCryptoStorePtr,
epoch_changes: SharedEpochChanges<B, Epoch>,
slot_notification_sinks: SlotNotificationSinks<B>,
config: Config,
}

impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeSlotWorker<B, C, E, I, SO> where
impl<B, C, E, I, Error, SO, BS> sc_consensus_slots::SimpleSlotWorker<B>
for BabeSlotWorker<B, C, E, I, SO, BS>
where
B: BlockT,
C: ProvideRuntimeApi<B> +
ProvideCache<B> +
Expand All @@ -513,6 +523,7 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeSlot
E::Proposer: Proposer<B, Error = Error, Transaction = sp_api::TransactionFor<C, B>>,
I: BlockImport<B, Transaction = sp_api::TransactionFor<C, B>> + Send + Sync + 'static,
SO: SyncOracle + Send + Clone,
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>>,
Error: std::error::Error + Send + From<ConsensusError> + From<I::Error> + 'static,
{
type EpochData = ViableEpochDescriptor<B::Hash, NumberFor<B>, Epoch>;
Expand Down Expand Up @@ -657,6 +668,23 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeSlot
self.force_authoring
}

fn should_backoff(&self, slot_number: u64, chain_head: &B::Header) -> bool {
if let Some(ref strategy) = self.backoff_authoring_blocks {
if let Ok(chain_head_slot) = find_pre_digest::<B>(chain_head)
.map(|digest| digest.slot_number())
{
return strategy.should_backoff(
*chain_head.number(),
chain_head_slot,
self.client.info().finalized_number,
slot_number,
self.logging_target(),
);
}
}
false
}

fn sync_oracle(&mut self) -> &mut Self::SyncOracle {
&mut self.sync_oracle
}
Expand Down
2 changes: 2 additions & 0 deletions client/consensus/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use sp_consensus_babe::{
make_transcript,
make_transcript_data,
};
use sc_consensus_slots::BackoffAuthoringOnFinalizedHeadLagging;
use sc_block_builder::{BlockBuilder, BlockBuilderProvider};
use sp_consensus::{
NoNetwork as DummyOracle, Proposal, RecordProof, AlwaysCanAuthor,
Expand Down Expand Up @@ -434,6 +435,7 @@ fn run_one_test(
sync_oracle: DummyOracle,
inherent_data_providers: data.inherent_data_providers.clone(),
force_authoring: false,
backoff_authoring_blocks: Some(BackoffAuthoringOnFinalizedHeadLagging::default()),
babe_link: data.link.clone(),
keystore,
can_author_with: sp_consensus::AlwaysCanAuthor,
Expand Down
1 change: 1 addition & 0 deletions client/consensus/slots/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ sc-client-api = { version = "2.0.0", path = "../../api" }
sp-core = { version = "2.0.0", path = "../../../primitives/core" }
sp-trie = { version = "2.0.0", path = "../../../primitives/trie" }
sp-application-crypto = { version = "2.0.0", path = "../../../primitives/application-crypto" }
sp-arithmetic = { version = "2.0.0", path = "../../../primitives/arithmetic" }
sp-blockchain = { version = "2.0.0", path = "../../../primitives/blockchain" }
sp-consensus-slots = { version = "0.8.0", path = "../../../primitives/consensus/slots" }
sp-runtime = { version = "2.0.0", path = "../../../primitives/runtime" }
Expand Down
Loading

0 comments on commit 1bde9ac

Please sign in to comment.