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

Simplify and Refactor pre_lock_filter #4980

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

Conversation

apfitzge
Copy link

Problem

  • pre_lock_filter has been unused so far
  • want to change from strict filter to allowing different actions to be taken. for example, skip w/o removing it from map

Summary of Changes

  • pre_lock_filter takes in transaction state
  • pre_lock_filter returns a PreLockFilterAction
  • PreLockFilterAction currently only supports simple pass-through

Future Plans

  1. Retry throttling
  • Add PreLockFilterAction::Skip which allows tx to be skipped, but not removed
  • pre_lock_filter will use the current bank slot to compare against a (soon to be introduced) latest retry slot to avoid retrying transactions too often
  1. IP filtering
  • if IP is sending us enough trash, we can quickly drop the transactions here

Fixes #

@apfitzge apfitzge marked this pull request as ready for review February 14, 2025 15:31
@apfitzge apfitzge requested a review from tao-stones February 14, 2025 15:31
tao-stones
tao-stones previously approved these changes Feb 18, 2025
Copy link

@tao-stones tao-stones left a comment

Choose a reason for hiding this comment

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

This could work; I was thinking another possibility of removing both filters to simplify Scheduler. pre_lock_filter could be do before scheduling, pre_graph_filter is bit tricky to determine if belongs to Scheduler. Let's get this moving, while working out details to do filtering early and separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants