-
Notifications
You must be signed in to change notification settings - Fork 153
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
Enhance Mempool performance #226
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… txs nor parent txs of the new transaction
…rphaned transactions (wip)
…s while revalidating high priority transactions
…event reentrance in mempool, broadcasting to and asking from peers
…orphan case (accepted txs will usually be orphan)
…apsulation logic since collections can be completely modified externally; while in tx pools it is important to make sure various internal collections are maintained consistently (for instance the `ready_transactions` field on `TransactionsPool` needs careful maintenance)
…ns in the case `remove_redeemers=false`. This is already done via `remove_transaction_from_sets` -> `transaction_pool.remove_transaction`. + a few minor changes
…s happens as part of: `remove_from_transaction_pool_and_update_orphans` -> `orphan_pool.update_orphans_after_transaction_removed` -> `orphan_pool.remove_redeemers_of`
… score after collection (bug fix)
michaelsutton
approved these changes
Oct 6, 2023
smartgoo
pushed a commit
to smartgoo/rusty-kaspa
that referenced
this pull request
Jun 18, 2024
* Split mempool atomic validate and insert transaction in 3 steps * Process tx relay flow received txs in batch * Use a single blocking task per MiningManagerProxy fn * Split parallel txs validation in chunks of max block mass * Abstract expire_low_priority_transactions into Pool trait * Making room in the mempool for a new transaction won't remove chained txs nor parent txs of the new transaction * Refine lock granularity on Mempool and Consensus while processing unorphaned transactions (wip) * Fix failing test * Enhance performance & refine lock granularity on Mempool and Consensus while revalidating high priority transactions * Comments * Fix upper bound of transactions chunk * Ensure a chunk has at least 1 tx * Prevent add twice the same tx to the mempool * Clear transaction entries before revalidation * Add some logs and comments * Add logs to debug transactions removals * On accepted block do not remove orphan tx redeemers * Add 2 TODOs * Fix a bug of high priority transactions being unexpectedly orphaned or rejected * Refactor transaction removal reason into an enum * Add an accepted transaction ids cache to the mempool and use it to prevent reentrance in mempool, broadcasting to and asking from peers * Improve the filtering of unknown transactions in tx relay * Enhance tx removal logging * Add mempool stats * Process new and unorphaned blocks in topological order * Run revalidation of HP txs in a dedicated task * Some profiling and debug logs * Run expiration of LP txs in a dedicated task * remove some stopwatch calls which were timing locks * crucial: fix exploding complexity of `handle_new_block_transactions`/`remove_transaction` * fixes in `on_new_block` * refactor block template cache into `Inner` * make `block_template_cache` a non-blocking call (never blocks) * Log build_block_template retries * While revalidating HP txs, only recheck transaction entries * Fix accepted count during revalidation * mempool bmk: use client pools + various improvements * Improve the topological sorting of transactions * Return transaction descendants BFS ordered + some optimizations * Group expiration and revalidation of mempool txs in one task * Refine the schedule of the cleaning task * ignore perf logs * maintain mempool ready transactions in a dedicated set * Bound the returned candidate transactions to a maximum * Reduces the max execution time of build block template * lint * Add mempool lock granularity to get_all_transactions * Restore block template cache lifetime & make it customizable in devnet-prealloc feature * Restore block template cache lifetime & make it customizable in devnet-prealloc feature * Relax a bit the BBT maximum attempts constraint * Refactor multiple `contained_by_txs` fns into one generic * Test selector transaction rejects & fix empty template returned by `select_transactions` upon selector reuse * Log some mempool metrics * Handle new block and then new block template * turn tx selector into an ongoing process with persistent state (wip: some tests are broken; selector is not used correctly by builder) * use tx selector for BBT (wip: virtual processor retry logic) * virtual processor selector retry logic * make BBT fallible by some selector criteria + comments and some docs * add an infallible mode to virtual processor `build_block_template()` * constants for tx selector successful decision * Add e-tps to logged mempool metrics * avoid realloc * Address review comments * Use number of ready txs in e-tps & enhance mempool lock * Ignore failing send for clean tokio shutdown * Log double spends * Log tx script cache stats (wip) * Ease atomic lock ordering & enhance counter updates * Enhance tx throughput stats log line * More robust management of cached data life cycle * Log mempool sampled instead of exact lengths * avoid passing consensus to orphan pool * rename ro `validate_transaction_unacceptance` and move to before the orphan case (accepted txs will usually be orphan) * rename `cleaning` -> `mempool_scanning` * keep intervals aligned using a round-up formula (rather than a loop) * design fix: avoid exposing full collections as mut. This violates encapsulation logic since collections can be completely modified externally; while in tx pools it is important to make sure various internal collections are maintained consistently (for instance the `ready_transactions` field on `TransactionsPool` needs careful maintenance) * minor: close all pool receivers on op error * `remove_transaction`: no need to manually update parent-child relations in the case `remove_redeemers=false`. This is already done via `remove_transaction_from_sets` -> `transaction_pool.remove_transaction`. + a few minor changes * encapsulate `remove_transaction_utxos` into `transaction_pool` * no need to `remove_redeemers_of` for the initial removed tx since this happens as part of: `remove_from_transaction_pool_and_update_orphans` -> `orphan_pool.update_orphans_after_transaction_removed` -> `orphan_pool.remove_redeemers_of` * inline `remove_from_transaction_pool_and_update_orphans` * remove redeemers of expired low-prio txs + register scan time and daa score after collection (bug fix) * change mempool monitor logs to debug * make tps logging more accurate * import bmk improvements from mempool-perf-stats branch * make `config.block_template_cache_lifetime` non-feature dependent --------- Co-authored-by: Michael Sutton <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses the issue of the mempool being a bottleneck in the first testnet-11 experiment @ 10 BPS.
The previous monolithical design did lock both the
Mempool
and to some extent the virtual processor during every call to its public functions, through theManager
that owns it, leading to some long delays. The new general design is to split the functions into simpler atomic steps logically protected by a lock on the mempool when the latter is implied. This approach has following consequences:Manager
owning theMempool
instance no longer is a simple pass-through locking and exposing mempool functions publicly. An important part of the mempool logic gets moved into the manager.Manager
function is no longer atomic.Another performance improvements are brought:
Validate and insert a single transaction
In order to bring a much finer lock granularity the function of validating and inserting a transaction into the mempool is split in 4 steps:
Mempool
)Mempool
)Mempool
)Validate and insert unorphaned transactions
Following any insert of a transaction into the mempool, some orphan transactions may be unorphaned. Here again we take advantage of the new design. On insertions, room is made on the fly if necessary and then we keep looping as long as new transactions are unorphaned (both new behaviors).
Validate and insert transactions in batch
This function is involved in the transaction relay flow, where a bunch of transactions gets broadcasted to peers. The batch is processed in a topological order by level of chained dependency (new behavior). For each level:
Handle the transactions of a block newly added to the DAG
Here again, a split in 3 steps adds granularity in the locks.
Revalidate high priority transactions
This is probably the most important bottleneck alleviation, together with tx relay broadcasting. This process occurs only every 30 seconds and for a node getting a very high rate of local transactions, it was implying a very computer intensive processing all under a single atomic lock of the mempool.
This gets redesigned as:
Since the process is now much more granular in how it locks both the mempool and the consensus, it may happen that some high priority transactions present at the beginning got removed (mined in a block, invalidated, etc.) during the (relatively long) process. The transaction id may or may not be returned as accepted depending on the sequence of events. But no impact is expected on the node. It will simply try to rebroadcast a transaction id that is no longer present and the peers requesting it will eventually get a
TransactionNotFound
, which may and does occur anyway for other reasons.The behavior changes compared to the golang version in that on transaction validation error the error is simply logged and the execution continues whereas in golang the execution halts returning the error.
Performance gains
Cautious note: an upcoming PR will add a benchmarking infrastructure making it possible to measure the gains in performance, hence revealing to which extent this new design actually alleviates the bottleneck issue.
Depending on the measures, some further adjustments might be needed.