Skip to content

Commit

Permalink
A0-3015: Finality rate metrics (#1521)
Browse files Browse the repository at this point in the history
# Description

Introduce `FinalityRateMetrics` - they measure the number of own
finalized blocks and the number of own hopeless blocks (our blocks which
are forking the finalized chain).

### Implementation details:
The interface consists of `{ report_own_imported, report_finalized }`.
`report_own_imported` is called when own block is imported and is added
to internal storage of `FinalityRateMetrics` object. `report_finalized`
is called when **any** finalized block appears and:
* if it is own block, it should have been added to internal storage when
reporting import - we count it as **own finalized**
* all blocks which are on the same level as the given newly finalized
block are removed from storage, and if they are not the given block,
they are counted as **own hopeless**.

## Type of change
- New feature (non-breaking change which adds functionality)

# Checklist:

<!-- delete when not applicable to your PR -->

- I have added tests
- I have made neccessary updates to the Infrastructure
- I have made corresponding changes to the existing documentation
- I have created new documentation
  • Loading branch information
woocash2 authored Dec 4, 2023
1 parent 9940089 commit c1eba3f
Show file tree
Hide file tree
Showing 14 changed files with 289 additions and 54 deletions.
18 changes: 4 additions & 14 deletions bin/node/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use std::{
use aleph_runtime::{self, opaque::Block, RuntimeApi};
use finality_aleph::{
run_validator_node, AlephBlockImport, AlephConfig, AllBlockMetrics, BlockImporter,
DefaultClock, Justification, JustificationTranslator, MillisecsPerBlock, Protocol,
ProtocolNaming, RateLimiterConfig, RedirectingBlockImport, SessionPeriod, SubstrateChainStatus,
SubstrateNetwork, SyncOracle, TimingBlockMetrics, TracingBlockImport, ValidatorAddressCache,
Justification, JustificationTranslator, MillisecsPerBlock, Protocol, ProtocolNaming,
RateLimiterConfig, RedirectingBlockImport, SessionPeriod, SubstrateChainStatus,
SubstrateNetwork, SyncOracle, TracingBlockImport, ValidatorAddressCache,
};
use futures::channel::mpsc;
use log::warn;
Expand Down Expand Up @@ -134,17 +134,7 @@ pub fn new_partial(
client.clone(),
);

let timing_metrics = match TimingBlockMetrics::new(config.prometheus_registry(), DefaultClock) {
Ok(timing_metrics) => timing_metrics,
Err(e) => {
warn!(
"Failed to register Prometheus block timing metrics: {:?}.",
e
);
TimingBlockMetrics::noop()
}
};
let metrics = AllBlockMetrics::new(timing_metrics);
let metrics = AllBlockMetrics::new(config.prometheus_registry());

let (justification_tx, justification_rx) = mpsc::unbounded();
let tracing_block_import = TracingBlockImport::new(client.clone(), metrics.clone());
Expand Down
2 changes: 1 addition & 1 deletion finality-aleph/src/block/mock/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ impl Finalizer<MockJustification> for Backend {
}

impl BlockImport<MockBlock> for Backend {
fn import_block(&mut self, block: MockBlock) {
fn import_block(&mut self, block: MockBlock, _own: bool) {
if !block.verify() {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion finality-aleph/src/block/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub trait Block: Clone + Codec + Debug + Send + Sync + 'static {
/// The block importer.
pub trait BlockImport<B>: Send + 'static {
/// Import the block.
fn import_block(&mut self, block: B);
fn import_block(&mut self, block: B, own: bool);
}

/// A facility for finalizing blocks using justifications.
Expand Down
16 changes: 11 additions & 5 deletions finality-aleph/src/block/substrate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::{
aleph_primitives::{Block, Header},
block::{Block as BlockT, BlockId, BlockImport, Header as HeaderT, UnverifiedHeader},
metrics::{AllBlockMetrics, Checkpoint},
TimingBlockMetrics,
};

mod chain_status;
Expand Down Expand Up @@ -66,7 +65,7 @@ impl BlockImporter {
pub fn new(importer: Box<dyn ImportQueueService<Block>>) -> Self {
Self {
importer,
metrics: AllBlockMetrics::new(TimingBlockMetrics::Noop),
metrics: AllBlockMetrics::new(None),
}
}

Expand All @@ -76,9 +75,15 @@ impl BlockImporter {
}

impl BlockImport<Block> for BlockImporter {
fn import_block(&mut self, block: Block) {
let origin = BlockOrigin::NetworkBroadcast;
fn import_block(&mut self, block: Block, own: bool) {
// We only need to distinguish between blocks produced by us and blocks incoming from the network
// for the purpose of running `FinalityRateMetrics`. We use `BlockOrigin` to make this distinction.
let origin = match own {
true => BlockOrigin::Own,
false => BlockOrigin::NetworkBroadcast,
};
let hash = block.header.hash();
let number = *block.header.number();
let incoming_block = IncomingBlock::<Block> {
hash,
header: Some(block.header),
Expand All @@ -91,7 +96,8 @@ impl BlockImport<Block> for BlockImporter {
import_existing: false,
state: None,
};
self.metrics.report_block(hash, Checkpoint::Importing);
self.metrics
.report_block(BlockId::new(hash, number), Checkpoint::Importing, Some(own));
self.importer.import_blocks(origin, vec![incoming_block]);
}
}
Expand Down
7 changes: 4 additions & 3 deletions finality-aleph/src/data_io/data_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,9 @@ impl<UH: UnverifiedHeader> DataProvider<UH> {
let data_to_propose = (*self.data_to_propose.lock()).take();

if let Some(data) = &data_to_propose {
let top_block = data.head_proposal.top_block();
self.metrics
.report_block(data.head_proposal.top_block().hash(), Checkpoint::Proposed);
.report_block(top_block, Checkpoint::Proposed, None);
debug!(target: "aleph-data-store", "Outputting {:?} in get_data", data);
};

Expand All @@ -360,7 +361,7 @@ mod tests {
client_chain_builder::ClientChainBuilder,
mocks::{aleph_data_from_blocks, THeader, TestClientBuilder, TestClientBuilderExt},
},
SessionBoundaryInfo, SessionId, SessionPeriod, TimingBlockMetrics,
SessionBoundaryInfo, SessionId, SessionPeriod,
};

const SESSION_LEN: u32 = 100;
Expand Down Expand Up @@ -391,7 +392,7 @@ mod tests {
client,
session_boundaries,
config,
AllBlockMetrics::new(TimingBlockMetrics::noop()),
AllBlockMetrics::new(None),
);

let (exit_chain_tracker_tx, exit_chain_tracker_rx) = oneshot::channel();
Expand Down
12 changes: 6 additions & 6 deletions finality-aleph/src/data_io/legacy/data_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,10 @@ impl DataProvider {
let data_to_propose = (*self.data_to_propose.lock()).take();

if let Some(data) = &data_to_propose {
self.metrics.report_block(
*data.head_proposal.branch.last().unwrap(),
Checkpoint::Proposed,
);
let number = data.head_proposal.number;
let hash = *data.head_proposal.branch.last().unwrap();
self.metrics
.report_block(BlockId::new(hash, number), Checkpoint::Proposed, None);
debug!(target: "aleph-data-store", "Outputting {:?} in get_data", data);
};

Expand All @@ -354,7 +354,7 @@ mod tests {
client_chain_builder::ClientChainBuilder,
mocks::{TestClientBuilder, TestClientBuilderExt},
},
SessionBoundaryInfo, SessionId, SessionPeriod, TimingBlockMetrics,
SessionBoundaryInfo, SessionId, SessionPeriod,
};

const SESSION_LEN: u32 = 100;
Expand Down Expand Up @@ -385,7 +385,7 @@ mod tests {
client,
session_boundaries,
config,
AllBlockMetrics::new(TimingBlockMetrics::noop()),
AllBlockMetrics::new(None),
);

let (exit_chain_tracker_tx, exit_chain_tracker_rx) = oneshot::channel();
Expand Down
3 changes: 2 additions & 1 deletion finality-aleph/src/finalization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ where
match &update_res {
Ok(_) => {
debug!(target: "aleph-finality", "Successfully finalized block with hash {:?} and number {:?}. Current best: #{:?}.", hash, number, status.best_number);
self.metrics.report_block(hash, Checkpoint::Finalized);
self.metrics
.report_block(block, Checkpoint::Finalized, None);
}
Err(_) => {
debug!(target: "aleph-finality", "Failed to finalize block with hash {:?} and number {:?}. Current best: #{:?}.", hash, number, status.best_number)
Expand Down
16 changes: 13 additions & 3 deletions finality-aleph/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use log::{debug, warn};
use sc_consensus::{
BlockCheckParams, BlockImport, BlockImportParams, ImportResult, JustificationImport,
};
use sp_consensus::Error as ConsensusError;
use sp_consensus::{BlockOrigin, Error as ConsensusError};
use sp_runtime::{traits::Header as HeaderT, Justification as SubstrateJustification};

use crate::{
Expand Down Expand Up @@ -58,14 +58,24 @@ where
block: BlockImportParams<Block>,
) -> Result<ImportResult, Self::Error> {
let post_hash = block.post_hash();
let number = *block.post_header().number();
let is_own = block.origin == BlockOrigin::Own;
// Self-created blocks are imported without using the import queue,
// so we need to report them here.
self.metrics.report_block(post_hash, Checkpoint::Importing);
self.metrics.report_block(
BlockId::new(post_hash, number),
Checkpoint::Importing,
Some(is_own),
);

let result = self.inner.import_block(block).await;

if let Ok(ImportResult::Imported(_)) = &result {
self.metrics.report_block(post_hash, Checkpoint::Imported);
self.metrics.report_block(
BlockId::new(post_hash, number),
Checkpoint::Imported,
Some(is_own),
);
}
result
}
Expand Down
2 changes: 1 addition & 1 deletion finality-aleph/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub use crate::{
},
import::{AlephBlockImport, RedirectingBlockImport, TracingBlockImport},
justification::AlephJustification,
metrics::{AllBlockMetrics, DefaultClock, TimingBlockMetrics},
metrics::{AllBlockMetrics, DefaultClock, FinalityRateMetrics, TimingBlockMetrics},
network::{
address_cache::{ValidatorAddressCache, ValidatorAddressingInfo},
Protocol, ProtocolNaming, SubstrateNetwork, SubstrateNetworkEventStream,
Expand Down
46 changes: 39 additions & 7 deletions finality-aleph/src/metrics/all_block.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,53 @@
use primitives::BlockHash;
use log::warn;
use substrate_prometheus_endpoint::Registry;

use super::{timing::DefaultClock, Checkpoint};
use crate::TimingBlockMetrics;
use super::{finality_rate::FinalityRateMetrics, timing::DefaultClock, Checkpoint};
use crate::{metrics::LOG_TARGET, BlockId, TimingBlockMetrics};

/// Wrapper around various block-related metrics.
#[derive(Clone)]
pub struct AllBlockMetrics {
timing_metrics: TimingBlockMetrics<DefaultClock>,
finality_rate_metrics: FinalityRateMetrics,
}

impl AllBlockMetrics {
pub fn new(timing_metrics: TimingBlockMetrics<DefaultClock>) -> Self {
AllBlockMetrics { timing_metrics }
pub fn new(registry: Option<&Registry>) -> Self {
let timing_metrics = match TimingBlockMetrics::new(registry, DefaultClock) {
Ok(timing_metrics) => timing_metrics,
Err(e) => {
warn!(
target: LOG_TARGET,
"Failed to register Prometheus block timing metrics: {:?}.", e
);
TimingBlockMetrics::Noop
}
};
let finality_rate_metrics = match FinalityRateMetrics::new(registry) {
Ok(finality_rate_metrics) => finality_rate_metrics,
Err(e) => {
warn!(
target: LOG_TARGET,
"Failed to register Prometheus finality rate metrics: {:?}.", e
);
FinalityRateMetrics::Noop
}
};
AllBlockMetrics {
timing_metrics,
finality_rate_metrics,
}
}

/// Triggers all contained block metrics.
pub fn report_block(&self, hash: BlockHash, checkpoint: Checkpoint) {
self.timing_metrics.report_block(hash, checkpoint);
pub fn report_block(&self, block_id: BlockId, checkpoint: Checkpoint, own: Option<bool>) {
self.timing_metrics
.report_block(block_id.hash(), checkpoint);
self.finality_rate_metrics.report_block(
block_id.hash(),
block_id.number(),
checkpoint,
own,
);
}
}
Loading

0 comments on commit c1eba3f

Please sign in to comment.