gnosis/lighthouse
1
0
Fork 0
mirror of https://github.com/sigp/lighthouse.git synced 2026-05-31 13:17:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

Move the BeaconProcessor into a new crate (#4435)

Browse Source 
*Replaces #4434. It is identical, but this PR has a smaller diff due to a curated commit history.*

## Issue Addressed

NA

## Proposed Changes

This PR moves the scheduling logic for the `BeaconProcessor` into a new crate in `beacon_node/beacon_processor`. Previously it existed in the `beacon_node/network` crate.

This addresses a circular-dependency problem where it's not possible to use the `BeaconProcessor` from the `beacon_chain` crate. The `network` crate depends on the `beacon_chain` crate (`network -> beacon_chain`), but importing the `BeaconProcessor` into the `beacon_chain` crate would create a circular dependancy of `beacon_chain -> network`.

The `BeaconProcessor` was designed to provide queuing and prioritized scheduling for messages from the network. It has proven to be quite valuable and I believe we'd make Lighthouse more stable and effective by using it elsewhere. In particular, I think we should use the `BeaconProcessor` for:

1. HTTP API requests.
1. Scheduled tasks in the `BeaconChain` (e.g., state advance).

Using the `BeaconProcessor` for these tasks would help prevent the BN from becoming overwhelmed and would also help it to prioritize operations (e.g., choosing to process blocks from gossip before responding to low-priority HTTP API requests).

## Additional Info

This PR is intended to have zero impact on runtime behaviour. It aims to simply separate the *scheduling* code (i.e., the `BeaconProcessor`) from the *business logic* in the `network` crate (i.e., the `Worker` impls). Future PRs (see #4462) can build upon these works to actually use the `BeaconProcessor` for more operations.

I've gone to some effort to use `git mv` to make the diff look more like "file was moved and modified" rather than "file was deleted and a new one added". This should reduce review burden and help maintain commit attribution.
This commit is contained in:
Paul Hauner
2023-07-10 07:45:54 +00:00
parent ea2420d193
commit c25825a539
28 changed files with 1743 additions and 1674 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
beacon_node/network/src/sync/backfill_sync/mod.rs
View File

@@ -8,7 +8,7 @@
//! If a batch fails, the backfill sync cannot progress. In this scenario, we mark the backfill
//! sync as failed, log an error and attempt to retry once a new peer joins the node.
use crate::beacon_processor::{ChainSegmentProcessId, WorkEvent as BeaconWorkEvent};
use crate::network_beacon_processor::ChainSegmentProcessId;
use crate::sync::manager::{BatchProcessResult, Id};
use crate::sync::network_context::SyncNetworkContext;
use crate::sync::range_sync::{
@@ -537,8 +537,8 @@ impl<T: BeaconChainTypes> BackFillSync<T> {
self.current_processing_batch = Some(batch_id);
if let Err(e) = network
.processor_channel()
.try_send(BeaconWorkEvent::chain_segment(process_id, blocks))
.beacon_processor()
.send_chain_segment(process_id, blocks)
{
crit!(self.log, "Failed to send backfill segment to processor."; "msg" => "process_batch",
"error" => %e, "batch" => self.processing_target);

20
beacon_node/network/src/sync/block_lookups/mod.rs
View File

@@ -2,6 +2,7 @@ use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::time::Duration;
use crate::network_beacon_processor::ChainSegmentProcessId;
use beacon_chain::{BeaconChainTypes, BlockError};
use fnv::FnvHashMap;
use lighthouse_network::{PeerAction, PeerId};
@@ -11,7 +12,6 @@ use smallvec::SmallVec;
use std::sync::Arc;
use store::{Hash256, SignedBeaconBlock};
use crate::beacon_processor::{ChainSegmentProcessId, WorkEvent};
use crate::metrics;
use self::parent_lookup::PARENT_FAIL_TOLERANCE;
@@ -542,8 +542,8 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
BlockProcessResult::Ok
| BlockProcessResult::Err(BlockError::BlockIsAlreadyKnown { .. }) => {
// Check if the beacon processor is available
let beacon_processor_send = match cx.processor_channel_if_enabled() {
Some(channel) => channel,
let beacon_processor = match cx.beacon_processor_if_enabled() {
Some(beacon_processor) => beacon_processor,
None => {
return trace!(
self.log,
@@ -555,7 +555,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
let (chain_hash, blocks, hashes, request) = parent_lookup.parts_for_processing();
let process_id = ChainSegmentProcessId::ParentLookup(chain_hash);
match beacon_processor_send.try_send(WorkEvent::chain_segment(process_id, blocks)) {
match beacon_processor.send_chain_segment(process_id, blocks) {
Ok(_) => {
self.processing_parent_lookups
.insert(chain_hash, (hashes, request));
@@ -664,11 +664,15 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
process_type: BlockProcessType,
cx: &mut SyncNetworkContext<T>,
) -> Result<(), ()> {
match cx.processor_channel_if_enabled() {
Some(beacon_processor_send) => {
match cx.beacon_processor_if_enabled() {
Some(beacon_processor) => {
trace!(self.log, "Sending block for processing"; "block" => ?block_root, "process" => ?process_type);
let event = WorkEvent::rpc_beacon_block(block_root, block, duration, process_type);
if let Err(e) = beacon_processor_send.try_send(event) {
if let Err(e) = beacon_processor.send_rpc_beacon_block(
block_root,
block,
duration,
process_type,
) {
error!(
self.log,
"Failed to send sync block to processor";

21
beacon_node/network/src/sync/block_lookups/tests.rs
View File

@@ -1,6 +1,6 @@
use std::sync::Arc;
use crate::beacon_processor::BeaconProcessorSend;
use crate::network_beacon_processor::NetworkBeaconProcessor;
use crate::service::RequestId;
use crate::sync::manager::RequestId as SyncId;
use crate::NetworkMessage;
@@ -9,18 +9,19 @@ use super::*;
use beacon_chain::builder::Witness;
use beacon_chain::eth1_chain::CachingEth1Backend;
use beacon_processor::WorkEvent;
use lighthouse_network::{NetworkGlobals, Request};
use slog::{Drain, Level};
use slot_clock::SystemTimeSlotClock;
use slot_clock::ManualSlotClock;
use store::MemoryStore;
use tokio::sync::mpsc;
use types::test_utils::{SeedableRng, TestRandom, XorShiftRng};
use types::MinimalEthSpec as E;
type T = Witness<SystemTimeSlotClock, CachingEth1Backend<E>, E, MemoryStore<E>, MemoryStore<E>>;
type T = Witness<ManualSlotClock, CachingEth1Backend<E>, E, MemoryStore<E>, MemoryStore<E>>;
struct TestRig {
beacon_processor_rx: mpsc::Receiver<WorkEvent<T>>,
beacon_processor_rx: mpsc::Receiver<WorkEvent<E>>,
network_rx: mpsc::UnboundedReceiver<NetworkMessage<E>>,
rng: XorShiftRng,
}
@@ -41,8 +42,10 @@ impl TestRig {
}
};
let (beacon_processor_tx, beacon_processor_rx) = mpsc::channel(100);
let (network_tx, network_rx) = mpsc::unbounded_channel();
let globals = Arc::new(NetworkGlobals::new_test_globals(Vec::new(), &log));
let (network_beacon_processor, beacon_processor_rx) =
NetworkBeaconProcessor::null_for_testing(globals);
let rng = XorShiftRng::from_seed([42; 16]);
let rig = TestRig {
beacon_processor_rx,
@@ -51,11 +54,9 @@ impl TestRig {
};
let bl = BlockLookups::new(log.new(slog::o!("component" => "block_lookups")));
let cx = {
let globals = Arc::new(NetworkGlobals::new_test_globals(Vec::new(), &log));
SyncNetworkContext::new(
network_tx,
globals,
BeaconProcessorSend(beacon_processor_tx),
Arc::new(network_beacon_processor),
log.new(slog::o!("component" => "network_context")),
)
};
@@ -102,7 +103,7 @@ impl TestRig {
fn expect_block_process(&mut self) {
match self.beacon_processor_rx.try_recv() {
Ok(work) => {
assert_eq!(work.work_type(), crate::beacon_processor::RPC_BLOCK);
assert_eq!(work.work_type(), beacon_processor::RPC_BLOCK);
}
other => panic!("Expected block process, found {:?}", other),
}
@@ -112,7 +113,7 @@ impl TestRig {
fn expect_parent_chain_process(&mut self) {
match self.beacon_processor_rx.try_recv() {
Ok(work) => {
assert_eq!(work.work_type(), crate::beacon_processor::CHAIN_SEGMENT);
assert_eq!(work.work_type(), beacon_processor::CHAIN_SEGMENT);
}
other => panic!("Expected chain segment process, found {:?}", other),
}

45
beacon_node/network/src/sync/manager.rs
View File

@@ -38,7 +38,7 @@ use super::block_lookups::BlockLookups;
use super::network_context::SyncNetworkContext;
use super::peer_sync_info::{remote_sync_type, PeerSyncType};
use super::range_sync::{RangeSync, RangeSyncType, EPOCHS_PER_BATCH};
use crate::beacon_processor::{BeaconProcessorSend, ChainSegmentProcessId};
use crate::network_beacon_processor::{ChainSegmentProcessId, NetworkBeaconProcessor};
use crate::service::NetworkMessage;
use crate::status::ToStatusMessage;
use beacon_chain::{BeaconChain, BeaconChainTypes, BlockError, EngineState};
@@ -159,9 +159,6 @@ pub struct SyncManager<T: BeaconChainTypes> {
/// A reference to the underlying beacon chain.
chain: Arc<BeaconChain<T>>,
/// A reference to the network globals and peer-db.
network_globals: Arc<NetworkGlobals<T::EthSpec>>,
/// A receiving channel sent by the message processor thread.
input_channel: mpsc::UnboundedReceiver<SyncMessage<T::EthSpec>>,
@@ -186,29 +183,22 @@ pub struct SyncManager<T: BeaconChainTypes> {
pub fn spawn<T: BeaconChainTypes>(
executor: task_executor::TaskExecutor,
beacon_chain: Arc<BeaconChain<T>>,
network_globals: Arc<NetworkGlobals<T::EthSpec>>,
network_send: mpsc::UnboundedSender<NetworkMessage<T::EthSpec>>,
beacon_processor_send: BeaconProcessorSend<T>,
beacon_processor: Arc<NetworkBeaconProcessor<T>>,
sync_recv: mpsc::UnboundedReceiver<SyncMessage<T::EthSpec>>,
log: slog::Logger,
) -> mpsc::UnboundedSender<SyncMessage<T::EthSpec>> {
) {
assert!(
MAX_REQUEST_BLOCKS >= T::EthSpec::slots_per_epoch() * EPOCHS_PER_BATCH,
"Max blocks that can be requested in a single batch greater than max allowed blocks in a single request"
);
// generate the message channel
let (sync_send, sync_recv) = mpsc::unbounded_channel::<SyncMessage<T::EthSpec>>();
// create an instance of the SyncManager
let network_globals = beacon_processor.network_globals.clone();
let mut sync_manager = SyncManager {
chain: beacon_chain.clone(),
network_globals: network_globals.clone(),
input_channel: sync_recv,
network: SyncNetworkContext::new(
network_send,
network_globals.clone(),
beacon_processor_send,
log.clone(),
),
network: SyncNetworkContext::new(network_send, beacon_processor, log.clone()),
range_sync: RangeSync::new(beacon_chain.clone(), log.clone()),
backfill_sync: BackFillSync::new(beacon_chain, network_globals, log.clone()),
block_lookups: BlockLookups::new(log.clone()),
@@ -218,10 +208,13 @@ pub fn spawn<T: BeaconChainTypes>(
// spawn the sync manager thread
debug!(log, "Sync Manager started");
executor.spawn(async move { Box::pin(sync_manager.main()).await }, "sync");
sync_send
}
impl<T: BeaconChainTypes> SyncManager<T> {
fn network_globals(&self) -> &NetworkGlobals<T::EthSpec> {
self.network.network_globals()
}
/* Input Handling Functions */
/// A peer has connected which has blocks that are unknown to us.
@@ -322,12 +315,12 @@ impl<T: BeaconChainTypes> SyncManager<T> {
let rpr = new_state.as_str();
// Drop the write lock
let update_sync_status = self
.network_globals
.network_globals()
.peers
.write()
.update_sync_status(peer_id, new_state.clone());
if let Some(was_updated) = update_sync_status {
let is_connected = self.network_globals.peers.read().is_connected(peer_id);
let is_connected = self.network_globals().peers.read().is_connected(peer_id);
if was_updated {
debug!(
self.log,
@@ -383,7 +376,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
let head = self.chain.best_slot();
let current_slot = self.chain.slot().unwrap_or_else(|_| Slot::new(0));
let peers = self.network_globals.peers.read();
let peers = self.network_globals().peers.read();
if current_slot >= head
&& current_slot.sub(head) <= (SLOT_IMPORT_TOLERANCE as u64)
&& head > 0
@@ -445,8 +438,8 @@ impl<T: BeaconChainTypes> SyncManager<T> {
},
};
let old_state = self.network_globals.set_sync_state(new_state);
let new_state = self.network_globals.sync_state.read();
let old_state = self.network_globals().set_sync_state(new_state);
let new_state = self.network_globals().sync_state.read().clone();
if !new_state.eq(&old_state) {
info!(self.log, "Sync state updated"; "old_state" => %old_state, "new_state" => %new_state);
// If we have become synced - Subscribe to all the core subnet topics
@@ -505,7 +498,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
}
SyncMessage::UnknownBlock(peer_id, block, block_root) => {
// If we are not synced or within SLOT_IMPORT_TOLERANCE of the block, ignore
if !self.network_globals.sync_state.read().is_synced() {
if !self.network_globals().sync_state.read().is_synced() {
let head_slot = self.chain.canonical_head.cached_head().head_slot();
let unknown_block_slot = block.slot();
@@ -519,7 +512,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
return;
}
}
if self.network_globals.peers.read().is_connected(&peer_id)
if self.network_globals().peers.read().is_connected(&peer_id)
&& self.network.is_execution_engine_online()
{
self.block_lookups
@@ -528,8 +521,8 @@ impl<T: BeaconChainTypes> SyncManager<T> {
}
SyncMessage::UnknownBlockHash(peer_id, block_hash) => {
// If we are not synced, ignore this block.
if self.network_globals.sync_state.read().is_synced()
&& self.network_globals.peers.read().is_connected(&peer_id)
if self.network_globals().sync_state.read().is_synced()
&& self.network_globals().peers.read().is_connected(&peer_id)
&& self.network.is_execution_engine_online()
{
self.block_lookups

29
beacon_node/network/src/sync/network_context.rs
View File

@@ -3,7 +3,7 @@
use super::manager::{Id, RequestId as SyncRequestId};
use super::range_sync::{BatchId, ChainId};
use crate::beacon_processor::BeaconProcessorSend;
use crate::network_beacon_processor::NetworkBeaconProcessor;
use crate::service::{NetworkMessage, RequestId};
use crate::status::ToStatusMessage;
use beacon_chain::{BeaconChainTypes, EngineState};
@@ -20,9 +20,6 @@ pub struct SyncNetworkContext<T: BeaconChainTypes> {
/// The network channel to relay messages to the Network service.
network_send: mpsc::UnboundedSender<NetworkMessage<T::EthSpec>>,
/// Access to the network global vars.
network_globals: Arc<NetworkGlobals<T::EthSpec>>,
/// A sequential ID for all RPC requests.
request_id: Id,
@@ -36,8 +33,8 @@ pub struct SyncNetworkContext<T: BeaconChainTypes> {
/// `beacon_processor_send`.
execution_engine_state: EngineState,
/// Channel to send work to the beacon processor.
beacon_processor_send: BeaconProcessorSend<T>,
/// Sends work to the beacon processor via a channel.
network_beacon_processor: Arc<NetworkBeaconProcessor<T>>,
/// Logger for the `SyncNetworkContext`.
log: slog::Logger,
@@ -46,25 +43,27 @@ pub struct SyncNetworkContext<T: BeaconChainTypes> {
impl<T: BeaconChainTypes> SyncNetworkContext<T> {
pub fn new(
network_send: mpsc::UnboundedSender<NetworkMessage<T::EthSpec>>,
network_globals: Arc<NetworkGlobals<T::EthSpec>>,
beacon_processor_send: BeaconProcessorSend<T>,
network_beacon_processor: Arc<NetworkBeaconProcessor<T>>,
log: slog::Logger,
) -> Self {
Self {
network_send,
execution_engine_state: EngineState::Online, // always assume `Online` at the start
network_globals,
request_id: 1,
range_requests: FnvHashMap::default(),
backfill_requests: FnvHashMap::default(),
beacon_processor_send,
network_beacon_processor,
log,
}
}
pub fn network_globals(&self) -> &NetworkGlobals<T::EthSpec> {
&self.network_beacon_processor.network_globals
}
/// Returns the Client type of the peer if known
pub fn client_type(&self, peer_id: &PeerId) -> Client {
self.network_globals
self.network_globals()
.peers
.read()
.peer_info(peer_id)
@@ -278,13 +277,13 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
})
}
pub fn processor_channel_if_enabled(&self) -> Option<&BeaconProcessorSend<T>> {
pub fn beacon_processor_if_enabled(&self) -> Option<&Arc<NetworkBeaconProcessor<T>>> {
self.is_execution_engine_online()
.then_some(&self.beacon_processor_send)
.then_some(&self.network_beacon_processor)
}
pub fn processor_channel(&self) -> &BeaconProcessorSend<T> {
&self.beacon_processor_send
pub fn beacon_processor(&self) -> &Arc<NetworkBeaconProcessor<T>> {
&self.network_beacon_processor
}
fn next_id(&mut self) -> Id {

10
beacon_node/network/src/sync/range_sync/chain.rs
View File

@@ -1,5 +1,5 @@
use super::batch::{BatchInfo, BatchProcessingResult, BatchState};
use crate::beacon_processor::{ChainSegmentProcessId, WorkEvent as BeaconWorkEvent};
use crate::network_beacon_processor::ChainSegmentProcessId;
use crate::sync::{
manager::Id, network_context::SyncNetworkContext, BatchOperationOutcome, BatchProcessResult,
};
@@ -294,8 +294,8 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
return Ok(KeepChain);
}
let beacon_processor_send = match network.processor_channel_if_enabled() {
Some(channel) => channel,
let beacon_processor = match network.beacon_processor_if_enabled() {
Some(beacon_processor) => beacon_processor,
None => return Ok(KeepChain),
};
@@ -317,9 +317,7 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
let process_id = ChainSegmentProcessId::RangeBatchId(self.id, batch_id);
self.current_processing_batch = Some(batch_id);
if let Err(e) =
beacon_processor_send.try_send(BeaconWorkEvent::chain_segment(process_id, blocks))
{
if let Err(e) = beacon_processor.send_chain_segment(process_id, blocks) {
crit!(self.log, "Failed to send chain segment to processor."; "msg" => "process_batch",
"error" => %e, "batch" => self.processing_target);
// This is unlikely to happen but it would stall syncing since the batch now has no

17
beacon_node/network/src/sync/range_sync/range.rs
View File

@@ -371,22 +371,23 @@ where
#[cfg(test)]
mod tests {
use crate::network_beacon_processor::NetworkBeaconProcessor;
use crate::service::RequestId;
use crate::NetworkMessage;
use super::*;
use crate::beacon_processor::{BeaconProcessorSend, WorkEvent as BeaconWorkEvent};
use beacon_chain::builder::Witness;
use beacon_chain::eth1_chain::CachingEth1Backend;
use beacon_chain::parking_lot::RwLock;
use beacon_chain::EngineState;
use beacon_processor::WorkEvent as BeaconWorkEvent;
use lighthouse_network::rpc::BlocksByRangeRequest;
use lighthouse_network::Request;
use lighthouse_network::{rpc::StatusMessage, NetworkGlobals};
use slog::{o, Drain};
use tokio::sync::mpsc;
use slot_clock::SystemTimeSlotClock;
use slot_clock::ManualSlotClock;
use std::collections::HashSet;
use std::sync::Arc;
use store::MemoryStore;
@@ -437,7 +438,7 @@ mod tests {
}
type TestBeaconChainType =
Witness<SystemTimeSlotClock, CachingEth1Backend<E>, E, MemoryStore<E>, MemoryStore<E>>;
Witness<ManualSlotClock, CachingEth1Backend<E>, E, MemoryStore<E>, MemoryStore<E>>;
fn build_log(level: slog::Level, enabled: bool) -> slog::Logger {
let decorator = slog_term::TermDecorator::new().build();
@@ -455,7 +456,7 @@ mod tests {
struct TestRig {
log: slog::Logger,
/// To check what does sync send to the beacon processor.
beacon_processor_rx: mpsc::Receiver<BeaconWorkEvent<TestBeaconChainType>>,
beacon_processor_rx: mpsc::Receiver<BeaconWorkEvent<E>>,
/// To set up different scenarios where sync is told about known/unkown blocks.
chain: Arc<FakeStorage>,
/// Needed by range to handle communication with the network.
@@ -583,7 +584,7 @@ mod tests {
fn expect_chain_segment(&mut self) {
match self.beacon_processor_rx.try_recv() {
Ok(work) => {
assert_eq!(work.work_type(), crate::beacon_processor::CHAIN_SEGMENT);
assert_eq!(work.work_type(), beacon_processor::CHAIN_SEGMENT);
}
other => panic!("Expected chain segment process, found {:?}", other),
}
@@ -593,17 +594,17 @@ mod tests {
fn range(log_enabled: bool) -> (TestRig, RangeSync<TestBeaconChainType, FakeStorage>) {
let chain = Arc::new(FakeStorage::default());
let log = build_log(slog::Level::Trace, log_enabled);
let (beacon_processor_tx, beacon_processor_rx) = mpsc::channel(10);
let range_sync = RangeSync::<TestBeaconChainType, FakeStorage>::new(
chain.clone(),
log.new(o!("component" => "range")),
);
let (network_tx, network_rx) = mpsc::unbounded_channel();
let globals = Arc::new(NetworkGlobals::new_test_globals(Vec::new(), &log));
let (network_beacon_processor, beacon_processor_rx) =
NetworkBeaconProcessor::null_for_testing(globals.clone());
let cx = SyncNetworkContext::new(
network_tx,
globals.clone(),
BeaconProcessorSend(beacon_processor_tx),
Arc::new(network_beacon_processor),
log.new(o!("component" => "network_context")),
);
let test_rig = TestRig {