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

Conversation

aborg-dev
Copy link
Contributor

@aborg-dev aborg-dev commented Apr 25, 2023

The PR introduces a limit to the size of the transaction pool for each shard and a logic that rejects transactions that go over this limit. The reasoning for this work is described in #3284.

To start, the limit will be disabled (effectively set to infinity) and this PR shouldn't have any effect. The node operators can override it with a config option. In the future, we will come up with a safe value to set by default (probably between 10 MiB - 1 GiB).

We also start with a simple option where the RPC client will not know if their transaction runs into this limit. We will need to rework this part in the future, but it will have to touch the transaction forwarding code in a non-trivial way, and I would prefer to work on this when we have a congestion test ready #8920.

Lastly, this PR adds some nuance to handling reintroducing transactions back to the pool after reorg or producing a chunk (reintroduce_transactions method), by acknowledging that not all transactions might have been included and logging the number of dropped transactions.

@aborg-dev aborg-dev added the A-congestion Work aimed at ensuring good system performance under congestion label Apr 25, 2023
@aborg-dev aborg-dev force-pushed the tp_limit_size branch 3 times, most recently from c3a56c9 to 3c518ff Compare May 2, 2023 12:50
@aborg-dev aborg-dev force-pushed the tp_limit_size branch 10 times, most recently from c1c49ba to 8ae3079 Compare May 11, 2023 10:47
@aborg-dev aborg-dev requested a review from nikurt May 11, 2023 11:00
@aborg-dev aborg-dev marked this pull request as ready for review May 11, 2023 11:00
@aborg-dev aborg-dev requested a review from a team as a code owner May 11, 2023 11:00
chain/pool/src/lib.rs Outdated Show resolved Hide resolved
chain/client/src/client.rs Outdated Show resolved Hide resolved

/// 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 {

chain/pool/src/lib.rs Outdated Show resolved Hide resolved
@near-bulldozer near-bulldozer bot merged commit cfa58b6 into near:master May 11, 2023
@aborg-dev aborg-dev deleted the tp_limit_size branch June 12, 2023 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-congestion Work aimed at ensuring good system performance under congestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants