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.

Sign up for GitHub

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

fix(sequencer): increase mempool removal cache size #1969

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(sequencer): increase mempool removal cache size #1969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 10 additions & 2 deletions crates/astria-sequencer/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Add more thorough unit tests for all actions [#1916](https://github.com/astriaorg/astria/pull/1916).
- Implement `BridgeTransfer` action [#1934](https://github.com/astriaorg/astria/pull/1934).

### Changed

- Bump MSRV to 1.83.0 [#1857](https://github.com/astriaorg/astria/pull/1857).
Expand All @@ -19,8 +24,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Remove events reporting on state storage creation [#1892](https://github.com/astriaorg/astria/pull/1892).
- Use bridge address to determine asset in bridge unlock cost estimation instead
of signer [#1905](https://github.com/astriaorg/astria/pull/1905).
- Add more thorough unit tests for all actions [#1916](https://github.com/astriaorg/astria/pull/1916).
- Implement `BridgeTransfer` action [#1934](https://github.com/astriaorg/astria/pull/1934).

### Fixed

- Increase mempool removal cache size to be greater than default CometBFT
mempool size [#1969](https://github.com/astriaorg/astria/pull/1969).

## [1.0.0] - 2024-10-25

Expand Down
16 changes: 13 additions & 3 deletions crates/astria-sequencer/src/mempool/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use tokio::{
use tracing::{
error,
instrument,
warn,
Level,
};
pub(crate) use transactions_container::InsertionError;
Expand Down Expand Up @@ -58,7 +59,7 @@ const TX_TTL: Duration = Duration::from_secs(240);
const MAX_PARKED_TXS_PER_ACCOUNT: usize = 15;
/// Max number of transactions to keep in the removal cache. Should be larger than the max number of
/// transactions allowed in the cometBFT mempool.
const REMOVAL_CACHE_SIZE: usize = 4096;
const REMOVAL_CACHE_SIZE: usize = 50_000;

/// `RemovalCache` is used to signal to `CometBFT` that a
/// transaction can be removed from the `CometBFT` mempool.
Expand Down Expand Up @@ -95,12 +96,21 @@ impl RemovalCache {
};

if self.remove_queue.len() == usize::from(self.max_size) {
// make space for the new transaction by removing the oldest transaction
// This should not happen if `REMOVAL_CACHE_SIZE` is >= CometBFT's configured mempool
// size.
//
// Make space for the new transaction by removing the oldest transaction.
let removed_tx = self
.remove_queue
.pop_front()
.expect("cache should contain elements");
// remove transaction from cache if it is present
warn!(
tx_hash = %telemetry::display::hex(&removed_tx),
removal_cache_size = REMOVAL_CACHE_SIZE,
"popped transaction from appside mempool removal cache, CometBFT will not remove \
this transaction from its mempool - removal cache size possibly too low"
);
// Remove transaction from cache if it is present.
self.cache.remove(&removed_tx);
}
self.remove_queue.push_back(tx_hash);
Expand Down
Loading