Skip to content

Commit

Permalink
[qs] batch fetcher with dedup
Browse files Browse the repository at this point in the history
  • Loading branch information
ibalajiarun committed Jan 17, 2025
1 parent 1d8460a commit 2bf1225
Show file tree
Hide file tree
Showing 14 changed files with 472 additions and 523 deletions.
29 changes: 5 additions & 24 deletions consensus/consensus-types/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use aptos_crypto::{
HashValue,
};
use aptos_crypto_derive::CryptoHasher;
use aptos_executor_types::ExecutorResult;
use aptos_infallible::Mutex;
use aptos_logger::prelude::*;
use aptos_types::{
Expand All @@ -28,7 +27,6 @@ use std::{
sync::Arc,
u64,
};
use tokio::sync::oneshot;

/// The round of a block is a consensus-internal counter, which starts with 0 and increases
/// monotonically. It is used for the protocol safety and liveness (please see the detailed
Expand Down Expand Up @@ -130,48 +128,38 @@ pub struct RejectedTransactionSummary {
#[derive(Debug)]
pub enum DataStatus {
Cached(Vec<SignedTransaction>),
Requested(
Vec<(
HashValue,
oneshot::Receiver<ExecutorResult<Vec<SignedTransaction>>>,
)>,
),
Requested,
}

impl DataStatus {
pub fn extend(&mut self, other: DataStatus) {
match (self, other) {
(DataStatus::Requested(v1), DataStatus::Requested(v2)) => v1.extend(v2),
(DataStatus::Requested, DataStatus::Requested) => {},
(_, _) => unreachable!(),
}
}

pub fn take(&mut self) -> DataStatus {
std::mem::replace(self, DataStatus::Requested(vec![]))
std::mem::replace(self, DataStatus::Requested)
}
}

#[derive(Deserialize, Serialize, Clone, Debug)]
pub struct ProofWithData {
pub proofs: Vec<ProofOfStore>,
#[serde(skip)]
pub status: Arc<Mutex<Option<DataStatus>>>,
}

impl PartialEq for ProofWithData {
fn eq(&self, other: &Self) -> bool {
self.proofs == other.proofs && Arc::as_ptr(&self.status) == Arc::as_ptr(&other.status)
self.proofs == other.proofs
}
}

impl Eq for ProofWithData {}

impl ProofWithData {
pub fn new(proofs: Vec<ProofOfStore>) -> Self {
Self {
proofs,
status: Arc::new(Mutex::new(None)),
}
Self { proofs }
}

pub fn empty() -> Self {
Expand All @@ -180,14 +168,7 @@ impl ProofWithData {

#[allow(clippy::unwrap_used)]
pub fn extend(&mut self, other: ProofWithData) {
let other_data_status = other.status.lock().as_mut().unwrap().take();
self.proofs.extend(other.proofs);
let mut status = self.status.lock();
if status.is_none() {
*status = Some(other_data_status);
} else {
status.as_mut().unwrap().extend(other_data_status);
}
}

pub fn len(&self) -> usize {
Expand Down
17 changes: 0 additions & 17 deletions consensus/consensus-types/src/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use crate::proof_of_store::{BatchInfo, ProofOfStore};
use anyhow::ensure;
use aptos_executor_types::ExecutorResult;
use aptos_infallible::Mutex;
use aptos_types::{transaction::SignedTransaction, PeerId};
use core::fmt;
use futures::{
Expand All @@ -15,7 +14,6 @@ use serde::{Deserialize, Serialize};
use std::{
fmt::Debug,
ops::{Deref, DerefMut},
sync::Arc,
};

pub type OptBatches = BatchPointer<BatchInfo>;
Expand All @@ -34,7 +32,6 @@ pub trait TDataInfo {

pub struct DataFetchFut {
pub iteration: u32,
pub responders: Vec<Arc<Mutex<Vec<PeerId>>>>,
pub fut: Shared<BoxFuture<'static, ExecutorResult<Vec<SignedTransaction>>>>,
}

Expand All @@ -61,8 +58,6 @@ impl DataFetchFut {
#[derive(Deserialize, Serialize, Clone, Debug)]
pub struct BatchPointer<T> {
pub batch_summary: Vec<T>,
#[serde(skip)]
pub data_fut: Arc<Mutex<Option<DataFetchFut>>>,
}

impl<T> BatchPointer<T>
Expand All @@ -72,21 +67,11 @@ where
pub fn new(metadata: Vec<T>) -> Self {
Self {
batch_summary: metadata,
data_fut: Arc::new(Mutex::new(None)),
}
}

pub fn extend(&mut self, other: BatchPointer<T>) {
let other_data_status = other.data_fut.lock().take().expect("must be initialized");
self.batch_summary.extend(other.batch_summary);
let mut status = self.data_fut.lock();
*status = match &mut *status {
None => Some(other_data_status),
Some(status) => {
status.extend(other_data_status);
return;
},
};
}

pub fn num_txns(&self) -> usize {
Expand Down Expand Up @@ -115,15 +100,13 @@ where
fn from(value: Vec<T>) -> Self {
Self {
batch_summary: value,
data_fut: Arc::new(Mutex::new(None)),
}
}
}

impl<T: PartialEq> PartialEq for BatchPointer<T> {
fn eq(&self, other: &Self) -> bool {
self.batch_summary == other.batch_summary
&& Arc::as_ptr(&self.data_fut) == Arc::as_ptr(&other.data_fut)
}
}

Expand Down
7 changes: 5 additions & 2 deletions consensus/src/block_storage/block_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,11 @@ impl BlockStore {
);

if let Some(payload) = block.payload() {
self.payload_manager
.prefetch_payload_data(payload, block.timestamp_usecs());
self.payload_manager.prefetch_payload_data(
payload,
block.author().expect("Payload block must have author"),
block.timestamp_usecs(),
);
}

let pipelined_block = PipelinedBlock::new_ordered(block.clone());
Expand Down
6 changes: 5 additions & 1 deletion consensus/src/block_storage/sync_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,11 @@ impl BlockStore {
for (i, block) in blocks.iter().enumerate() {
assert_eq!(block.id(), quorum_certs[i].certified_block().id());
if let Some(payload) = block.payload() {
payload_manager.prefetch_payload_data(payload, block.timestamp_usecs());
payload_manager.prefetch_payload_data(
payload,
block.author().expect("payload block must have author"),
block.timestamp_usecs(),
);
}
}

Expand Down
3 changes: 2 additions & 1 deletion consensus/src/consensus_observer/observer/active_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use crate::{
publisher::consensus_publisher::ConsensusPublisher,
},
payload_manager::{
ConsensusObserverPayloadManager, DirectMempoolPayloadManager, TPayloadManager,
co_payload_manager::ConsensusObserverPayloadManager,
direct_mempool_payload_manager::DirectMempoolPayloadManager, TPayloadManager,
},
state_replication::StateComputerCommitCallBackType,
};
Expand Down
7 changes: 5 additions & 2 deletions consensus/src/dag/dag_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,11 @@ impl DagStore {
self.storage.save_certified_node(&node)?;

debug!("Added node {}", node.id());
self.payload_manager
.prefetch_payload_data(node.payload(), node.metadata().timestamp());
self.payload_manager.prefetch_payload_data(
node.payload(),
*node.author(),
node.metadata().timestamp(),
);

self.dag.write().add_validated_node(node)
}
Expand Down
11 changes: 8 additions & 3 deletions consensus/src/epoch_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ use crate::{
payload_client::{
mixed::MixedPayloadClient, user::quorum_store_client::QuorumStoreClient, PayloadClient,
},
payload_manager::{DirectMempoolPayloadManager, TPayloadManager},
payload_manager::{
direct_mempool_payload_manager::DirectMempoolPayloadManager, TPayloadManager,
},
persistent_liveness_storage::{LedgerRecoveryData, PersistentLivenessStorage, RecoveryData},
pipeline::execution_client::TExecutionClient,
quorum_store::{
Expand Down Expand Up @@ -1646,8 +1648,11 @@ impl<P: OnChainConfigProvider> EpochManager<P> {
proposal_event @ VerifiedEvent::ProposalMsg(_) => {
if let VerifiedEvent::ProposalMsg(p) = &proposal_event {
if let Some(payload) = p.proposal().payload() {
payload_manager
.prefetch_payload_data(payload, p.proposal().timestamp_usecs());
payload_manager.prefetch_payload_data(
payload,
p.proposer(),
p.proposal().timestamp_usecs(),
);
}
pending_blocks.lock().insert_block(p.proposal().clone());
}
Expand Down
115 changes: 115 additions & 0 deletions consensus/src/payload_manager/co_payload_manager.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use crate::{
consensus_observer::{
network::observer_message::ConsensusObserverMessage,
observer::payload_store::BlockPayloadStatus,
publisher::consensus_publisher::ConsensusPublisher,
},
payload_manager::TPayloadManager,
};
use aptos_bitvec::BitVec;
use aptos_consensus_types::{
block::Block,
common::{Author, Payload, Round},
};
use aptos_crypto::HashValue;
use aptos_executor_types::{ExecutorError::InternalError, *};
use aptos_infallible::Mutex;
use aptos_types::transaction::SignedTransaction;
use async_trait::async_trait;
use std::{
collections::{btree_map::Entry, BTreeMap},
sync::Arc,
};

/// Returns the transactions for the consensus observer payload manager
async fn get_transactions_for_observer(
block: &Block,
block_payloads: &Arc<Mutex<BTreeMap<(u64, Round), BlockPayloadStatus>>>,
consensus_publisher: &Option<Arc<ConsensusPublisher>>,
) -> ExecutorResult<(Vec<SignedTransaction>, Option<u64>)> {
// The data should already be available (as consensus observer will only ever
// forward a block to the executor once the data has been received and verified).
let block_payload = match block_payloads.lock().entry((block.epoch(), block.round())) {
Entry::Occupied(mut value) => match value.get_mut() {
BlockPayloadStatus::AvailableAndVerified(block_payload) => block_payload.clone(),
BlockPayloadStatus::AvailableAndUnverified(_) => {
// This shouldn't happen (the payload should already be verified)
let error = format!(
"Payload data for block epoch {}, round {} is unverified!",
block.epoch(),
block.round()
);
return Err(InternalError { error });
},
},
Entry::Vacant(_) => {
// This shouldn't happen (the payload should already be present)
let error = format!(
"Missing payload data for block epoch {}, round {}!",
block.epoch(),
block.round()
);
return Err(InternalError { error });
},
};

// If the payload is valid, publish it to any downstream observers
let transaction_payload = block_payload.transaction_payload();
if let Some(consensus_publisher) = consensus_publisher {
let message = ConsensusObserverMessage::new_block_payload_message(
block.gen_block_info(HashValue::zero(), 0, None),
transaction_payload.clone(),
);
consensus_publisher.publish_message(message);
}

// Return the transactions and the transaction limit
Ok((
transaction_payload.transactions(),
transaction_payload.limit(),
))
}

pub struct ConsensusObserverPayloadManager {
txns_pool: Arc<Mutex<BTreeMap<(u64, Round), BlockPayloadStatus>>>,
consensus_publisher: Option<Arc<ConsensusPublisher>>,
}

impl ConsensusObserverPayloadManager {
pub fn new(
txns_pool: Arc<Mutex<BTreeMap<(u64, Round), BlockPayloadStatus>>>,
consensus_publisher: Option<Arc<ConsensusPublisher>>,
) -> Self {
Self {
txns_pool,
consensus_publisher,
}
}
}

#[async_trait]
impl TPayloadManager for ConsensusObserverPayloadManager {
fn notify_commit(&self, _block_timestamp: u64, _payloads: Vec<Payload>) {
// noop
}

fn prefetch_payload_data(&self, _payload: &Payload, _author: Author, _timestamp: u64) {
// noop
}

fn check_payload_availability(&self, _block: &Block) -> Result<(), BitVec> {
unreachable!("this method isn't used in ConsensusObserver")
}

async fn get_transactions(
&self,
block: &Block,
_block_signers: Option<BitVec>,
) -> ExecutorResult<(Vec<SignedTransaction>, Option<u64>)> {
return get_transactions_for_observer(block, &self.txns_pool, &self.consensus_publisher)
.await;
}
}
Loading

0 comments on commit 2bf1225

Please sign in to comment.