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

fatxpool: handling limits and priorities improvements #6405

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

michalkucharczyk
Copy link
Contributor

@michalkucharczyk michalkucharczyk commented Nov 7, 2024

This PR provides a number of improvements around handling limits and priorities in the fork-aware transaction pool.

Notes to reviewers.

Following are the notable changes:

  1. Better support for Usurped transactions

    When any view reports an Usurped transaction (replaced by other with higher priority) it is removed from all the views (also inactive). Removal is implemented by simply submitting usurper transaction to all the views. It is also ensured that usurped tx will not sneak into the view_store in newly created view (this is why ViewStore::pending_txs_replacements was added).

  2. TimedTransactionSource introduced:

    Every view now has an information when the transaction entered the pool. Enforce limits (now only for future txs) uses this timestamp to find worst transactions. Having common timestamp ensures coherent assessment of the transaction's importance across different views. This also could later be used to select which ready transaction shall be dropped.

  3. DroppedWatcher: improved logic for future transactions

    For future transaction - if the last referencing view is removed, the transaction will be dropped from the pool. This prevents future unincluded and un-promoted transactions from staying in the pool for long time.

And some minor changes:

  1. simplified the flow in update_view_with_mempool (code duplication + minor bug fix).
  2. graph::BasePool: handling priorities for future transaction improved (previously transaction with lower prio was reported as failed),
  3. graph::listener: dedicated limit_enforced/usurped/dropped calls added,
  4. flaky test fixed
  5. new tests added,

related to: #5809

@michalkucharczyk michalkucharczyk changed the title fatxpool: improvements in handling of priorities fatxpool:limits and priorities improvements Nov 14, 2024
@michalkucharczyk michalkucharczyk changed the title fatxpool:limits and priorities improvements fatxpool: limits and priorities improvements Nov 14, 2024
@michalkucharczyk michalkucharczyk changed the title fatxpool: limits and priorities improvements fatxpool: handling limits and priorities improvements Nov 14, 2024
@michalkucharczyk
Copy link
Contributor Author

/cmd prdoc --bump major --audience node_dev

@michalkucharczyk michalkucharczyk added R0-silent Changes should not be mentioned in any release notes T0-node This PR/Issue is related to the topic “node”. labels Nov 14, 2024
@michalkucharczyk michalkucharczyk marked this pull request as ready for review November 14, 2024 17:11
@michalkucharczyk michalkucharczyk requested review from bkchr and a team November 15, 2024 12:36
@michalkucharczyk michalkucharczyk requested review from iulianbarbu and a team and removed request for iulianbarbu November 20, 2024 12:54
@michalkucharczyk michalkucharczyk requested a review from a team November 20, 2024 14:20
Copy link
Contributor

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

LGTM! Not entirely familiar with the code, some minor needs that could all be ignored /followed up later if need be 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants