Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Introduce a limit to transaction pool size #8970

Merged
merged 6 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feature: Introduce a limit to transaction pool size
- Add enum for different transaction outcomes
- Introduce transaction pool size accounting
  • Loading branch information
aborg-dev committed May 11, 2023
commit e39e52484417ae3f10bc3baa705140c8a3114f2e
39 changes: 34 additions & 5 deletions chain/chunks/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,55 @@ pub enum ShardsManagerResponse {
ChunkHeaderReadyForInclusion { chunk_header: ShardChunkHeader, chunk_producer: AccountId },
}

#[derive(Debug)]
pub enum InsertTransactionResult {
/// Transaction was successfully inserted.
Success,
/// Transaction is already in the pool.
Duplicate,
/// Not enough space to fit the transaction.
NoSpaceLeft,
}

pub struct ShardedTransactionPool {
tx_pools: HashMap<ShardId, TransactionPool>,

/// Useful to make tests deterministic and reproducible,
/// while keeping the security of randomization of transactions in pool
rng_seed: RngSeed,

/// If set, new transactions that bring the size of the pool over this limit will be rejected.
/// The size is tracked and enforced separately for each shard.
pool_size_limit: Option<u64>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know what is the largest valid transaction?

Copy link
Contributor Author

@aborg-dev aborg-dev May 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen the number "4MiB" mentioned a few times, e.g. in #8936 and

max_transaction_size 4_194_304
and
if transaction_size > max_transaction_size {

}

impl ShardedTransactionPool {
pub fn new(rng_seed: RngSeed) -> Self {
pub fn new(rng_seed: RngSeed, pool_size_limit: Option<u64>) -> Self {
TransactionPool::init_metrics();
Self { tx_pools: HashMap::new(), rng_seed }
Self { tx_pools: HashMap::new(), rng_seed, pool_size_limit }
}

pub fn get_pool_iterator(&mut self, shard_id: ShardId) -> Option<PoolIteratorWrapper<'_>> {
self.tx_pools.get_mut(&shard_id).map(|pool| pool.pool_iterator())
}

/// Returns true if transaction is not in the pool before call
pub fn insert_transaction(&mut self, shard_id: ShardId, tx: SignedTransaction) -> bool {
self.pool_for_shard(shard_id).insert_transaction(tx)
/// Tries to insert the transaction into the pool for a given shard.
pub fn insert_transaction(
&mut self,
shard_id: ShardId,
tx: SignedTransaction,
) -> InsertTransactionResult {
if let Some(limit) = self.pool_size_limit {
if self.pool_for_shard(shard_id).transaction_size() > limit {
return InsertTransactionResult::NoSpaceLeft;
}
}

if !self.pool_for_shard(shard_id).insert_transaction(tx) {
return InsertTransactionResult::Duplicate;
}

InsertTransactionResult::Success
}

pub fn remove_transactions(&mut self, shard_id: ShardId, transactions: &[SignedTransaction]) {
Expand Down Expand Up @@ -81,6 +109,7 @@ impl ShardedTransactionPool {
shard_id: ShardId,
transactions: &[SignedTransaction],
) {
// TODO(akashin): Enforce size limit here as well.
self.pool_for_shard(shard_id).reintroduce_transactions(transactions.to_vec());
}
}
Expand Down
18 changes: 15 additions & 3 deletions chain/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ impl Client {
chain.store(),
chain_config.background_migration_threads,
)?;
let sharded_tx_pool = ShardedTransactionPool::new(rng_seed);
let sharded_tx_pool =
ShardedTransactionPool::new(rng_seed, config.transaction_pool_size_limit);
let sync_status = SyncStatus::AwaitingPeers;
let genesis_block = chain.genesis_block();
let epoch_sync = EpochSync::new(
Expand Down Expand Up @@ -2010,8 +2011,19 @@ impl Client {
} else {
// Transactions only need to be recorded if the node is a validator.
if me.is_some() {
self.sharded_tx_pool.insert_transaction(shard_id, tx.clone());
trace!(target: "client", shard_id, "Recorded a transaction.");
match self.sharded_tx_pool.insert_transaction(shard_id, tx.clone()) {
near_chunks::client::InsertTransactionResult::Success => {
trace!(target: "client", shard_id, "Recorded a transaction.");
}
near_chunks::client::InsertTransactionResult::Duplicate => {
// TODO(akashin): Decide what to do in this case.
return Err(Error::Other("Duplicate transaction inserted".to_string()));
}
near_chunks::client::InsertTransactionResult::NoSpaceLeft => {
// TODO(akashin): Introduce a dedicated error code.
return Err(Error::Other("No space left".to_string()));
}
}
}

// Active validator:
Expand Down
4 changes: 4 additions & 0 deletions core/chain-configs/src/client_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,9 @@ pub struct ClientConfig {
pub state_sync_enabled: bool,
/// Options for syncing state.
pub state_sync: StateSyncConfig,
/// Limit of the size of per-shard transaction pool measured in bytes. If not set, the size
/// will be unbounded.
pub transaction_pool_size_limit: Option<u64>,
}

impl ClientConfig {
Expand Down Expand Up @@ -323,6 +326,7 @@ impl ClientConfig {
flat_storage_creation_period: Duration::from_secs(1),
state_sync_enabled: false,
state_sync: StateSyncConfig::default(),
transaction_pool_size_limit: None,
}
}
}
1 change: 1 addition & 0 deletions nearcore/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,7 @@ impl NearConfig {
flat_storage_creation_period: config.store.flat_storage_creation_period,
state_sync_enabled: config.state_sync_enabled.unwrap_or(false),
state_sync: config.state_sync.unwrap_or_default(),
transaction_pool_size_limit: None,
},
network_config: NetworkConfig::new(
config.network,
Expand Down