From 96b7cec1ceb8df01b59ebaf353a8b3b60132a97c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Silva?=
<123550+andresilva@users.noreply.github.com>
Date: Thu, 23 Apr 2020 08:42:53 +0100
Subject: [PATCH] slots: fix slot lenience methods (#5742)
* slots: extract slot lenience from babe and aura
* slots: add tests for slot lenience
* slots: fix comment in test
---
substrate/client/consensus/aura/src/lib.rs | 29 ++---
substrate/client/consensus/babe/src/lib.rs | 36 +++---
substrate/client/consensus/slots/src/lib.rs | 117 ++++++++++++++++++++
3 files changed, 145 insertions(+), 37 deletions(-)
diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs
index 56674546d37..038e9e458cf 100644
--- a/substrate/client/consensus/aura/src/lib.rs
+++ b/substrate/client/consensus/aura/src/lib.rs
@@ -299,27 +299,28 @@ impl<B, C, E, I, P, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for AuraW
fn proposing_remaining_duration(
&self,
head: &B::Header,
- slot_info: &SlotInfo
+ slot_info: &SlotInfo,
) -> Option<std::time::Duration> {
- // never give more than 20 times more lenience.
- const BACKOFF_CAP: u64 = 20;
-
let slot_remaining = self.slot_remaining_duration(slot_info);
+
let parent_slot = match find_pre_digest::<B, P>(head) {
Err(_) => return Some(slot_remaining),
Ok(d) => d,
};
- // we allow a lenience of the number of slots since the head of the
- // chain was produced, minus 1 (since there is always a difference of at least 1)
- //
- // linear back-off.
- // in normal cases we only attempt to issue blocks up to the end of the slot.
- // when the chain has been stalled for a few slots, we give more lenience.
- let slot_lenience = slot_info.number.saturating_sub(parent_slot + 1);
- let slot_lenience = std::cmp::min(slot_lenience, BACKOFF_CAP);
- let slot_lenience = Duration::from_secs(slot_lenience * slot_info.duration);
- Some(slot_lenience + slot_remaining)
+ if let Some(slot_lenience) =
+ sc_consensus_slots::slot_lenience_exponential(parent_slot, slot_info)
+ {
+ debug!(target: "aura",
+ "No block for {} slots. Applying linear lenience of {}s",
+ slot_info.number.saturating_sub(parent_slot + 1),
+ slot_lenience.as_secs(),
+ );
+
+ Some(slot_remaining + slot_lenience)
+ } else {
+ Some(slot_remaining)
+ }
}
}
diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs
index 092bf8153b9..59b49541821 100644
--- a/substrate/client/consensus/babe/src/lib.rs
+++ b/substrate/client/consensus/babe/src/lib.rs
@@ -510,38 +510,28 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeWork
fn proposing_remaining_duration(
&self,
head: &B::Header,
- slot_info: &SlotInfo
+ slot_info: &SlotInfo,
) -> Option<std::time::Duration> {
- // never give more than 2^this times the lenience.
- const BACKOFF_CAP: u64 = 8;
-
- // how many slots it takes before we double the lenience.
- const BACKOFF_STEP: u64 = 2;
-
let slot_remaining = self.slot_remaining_duration(slot_info);
+
let parent_slot = match find_pre_digest::<B>(head) {
Err(_) => return Some(slot_remaining),
Ok(d) => d.slot_number(),
};
- // we allow a lenience of the number of slots since the head of the
- // chain was produced, minus 1 (since there is always a difference of at least 1)
- //
- // exponential back-off.
- // in normal cases we only attempt to issue blocks up to the end of the slot.
- // when the chain has been stalled for a few slots, we give more lenience.
- let slot_lenience = slot_info.number.saturating_sub(parent_slot + 1);
-
- let slot_lenience = std::cmp::min(slot_lenience, BACKOFF_CAP);
- let slot_duration = slot_info.duration << (slot_lenience / BACKOFF_STEP);
+ if let Some(slot_lenience) =
+ sc_consensus_slots::slot_lenience_exponential(parent_slot, slot_info)
+ {
+ debug!(target: "babe",
+ "No block for {} slots. Applying exponential lenience of {}s",
+ slot_info.number.saturating_sub(parent_slot + 1),
+ slot_lenience.as_secs(),
+ );
- if slot_lenience >= 1 {
- debug!(target: "babe", "No block for {} slots. Applying 2^({}/{}) lenience",
- slot_lenience, slot_lenience, BACKOFF_STEP);
+ Some(slot_remaining + slot_lenience)
+ } else {
+ Some(slot_remaining)
}
-
- let slot_lenience = Duration::from_secs(slot_duration);
- Some(slot_lenience + slot_remaining)
}
}
diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs
index 2a0739a831b..611e0fbb7b8 100644
--- a/substrate/client/consensus/slots/src/lib.rs
+++ b/substrate/client/consensus/slots/src/lib.rs
@@ -483,3 +483,120 @@ impl<T: Clone> SlotDuration<T> {
self.0.clone()
}
}
+
+/// Calculate a slot duration lenience based on the number of missed slots from current
+/// to parent. If the number of skipped slots is greated than 0 this method will apply
+/// an exponential backoff of at most `2^7 * slot_duration`, if no slots were skipped
+/// this method will return `None.`
+pub fn slot_lenience_exponential(parent_slot: u64, slot_info: &SlotInfo) -> Option<Duration> {
+ // never give more than 2^this times the lenience.
+ const BACKOFF_CAP: u64 = 7;
+
+ // how many slots it takes before we double the lenience.
+ const BACKOFF_STEP: u64 = 2;
+
+ // we allow a lenience of the number of slots since the head of the
+ // chain was produced, minus 1 (since there is always a difference of at least 1)
+ //
+ // exponential back-off.
+ // in normal cases we only attempt to issue blocks up to the end of the slot.
+ // when the chain has been stalled for a few slots, we give more lenience.
+ let skipped_slots = slot_info.number.saturating_sub(parent_slot + 1);
+
+ if skipped_slots == 0 {
+ None
+ } else {
+ let slot_lenience = skipped_slots / BACKOFF_STEP;
+ let slot_lenience = std::cmp::min(slot_lenience, BACKOFF_CAP);
+ let slot_lenience = 1 << slot_lenience;
+ Some(Duration::from_millis(slot_lenience * slot_info.duration))
+ }
+}
+
+/// Calculate a slot duration lenience based on the number of missed slots from current
+/// to parent. If the number of skipped slots is greated than 0 this method will apply
+/// a linear backoff of at most `20 * slot_duration`, if no slots were skipped
+/// this method will return `None.`
+pub fn slot_lenience_linear(parent_slot: u64, slot_info: &SlotInfo) -> Option<Duration> {
+ // never give more than 20 times more lenience.
+ const BACKOFF_CAP: u64 = 20;
+
+ // we allow a lenience of the number of slots since the head of the
+ // chain was produced, minus 1 (since there is always a difference of at least 1)
+ //
+ // linear back-off.
+ // in normal cases we only attempt to issue blocks up to the end of the slot.
+ // when the chain has been stalled for a few slots, we give more lenience.
+ let skipped_slots = slot_info.number.saturating_sub(parent_slot + 1);
+
+ if skipped_slots == 0 {
+ None
+ } else {
+ let slot_lenience = std::cmp::min(skipped_slots, BACKOFF_CAP);
+ Some(Duration::from_millis(slot_lenience * slot_info.duration))
+ }
+}
+
+#[cfg(test)]
+mod test {
+ use std::time::{Duration, Instant};
+
+ const SLOT_DURATION: Duration = Duration::from_millis(6000);
+
+ fn slot(n: u64) -> super::slots::SlotInfo {
+ super::slots::SlotInfo {
+ number: n,
+ last_number: n - 1,
+ duration: SLOT_DURATION.as_millis() as u64,
+ timestamp: Default::default(),
+ inherent_data: Default::default(),
+ ends_at: Instant::now(),
+ }
+ }
+
+ #[test]
+ fn linear_slot_lenience() {
+ // if no slots are skipped there should be no lenience
+ assert_eq!(super::slot_lenience_linear(1, &slot(2)), None);
+
+ // otherwise the lenience is incremented linearly with
+ // the number of skipped slots.
+ for n in 3..=22 {
+ assert_eq!(
+ super::slot_lenience_linear(1, &slot(n)),
+ Some(SLOT_DURATION * (n - 2) as u32),
+ );
+ }
+
+ // but we cap it to a maximum of 20 slots
+ assert_eq!(
+ super::slot_lenience_linear(1, &slot(23)),
+ Some(SLOT_DURATION * 20),
+ );
+ }
+
+ #[test]
+ fn exponential_slot_lenience() {
+ // if no slots are skipped there should be no lenience
+ assert_eq!(super::slot_lenience_exponential(1, &slot(2)), None);
+
+ // otherwise the lenience is incremented exponentially every two slots
+ for n in 3..=17 {
+ assert_eq!(
+ super::slot_lenience_exponential(1, &slot(n)),
+ Some(SLOT_DURATION * 2u32.pow((n / 2 - 1) as u32)),
+ );
+ }
+
+ // but we cap it to a maximum of 14 slots
+ assert_eq!(
+ super::slot_lenience_exponential(1, &slot(18)),
+ Some(SLOT_DURATION * 2u32.pow(7)),
+ );
+
+ assert_eq!(
+ super::slot_lenience_exponential(1, &slot(19)),
+ Some(SLOT_DURATION * 2u32.pow(7)),
+ );
+ }
+}
--
GitLab