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

chore(events): improve perf for parallel event filter matching #12603

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

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Oct 16, 2024

Blocked until #12421 is merged and we can do this against master.

I started writing up a good-first-issue ticket for this but it eneded up being easier to write the code than describe what needed doing.

  1. Cache address look-ups for the given tipset across filters
  2. Run the filters in an errgroup
  3. Removed the bool return argument and just use address.Undef, I believe there's no case where this is an acceptable value

Filters can be installed via eth_newFilter, eth_subscribe and SubscribeActorEventsRaw. All filters installed have CollectEvents run on them for each new tipset and revert. So this can get expensive if you're an RPC provider who allows users to install these things and watch for live events.

@BigLep
Copy link
Member

BigLep commented Oct 28, 2024

@rvagg : is this something you're hoping to see land for 1.31.0?

@rvagg
Copy link
Member Author

rvagg commented Oct 28, 2024

Yes, but I'd like chainindexer to land first, this can go in separately. Like #12623 I think they deserve separate call-outs in the CHANGELOG.

Base automatically changed from feat/msg-eth-tx-index to master October 31, 2024 09:58
@rvagg rvagg force-pushed the rvagg/parallel-event-filters branch from 746e749 to 35a9852 Compare November 4, 2024 02:56
1. Cache address look-ups for the given tipset across filters
2. Run the filters in an errgroup
@rvagg rvagg force-pushed the rvagg/parallel-event-filters branch from 35a9852 to e01d209 Compare November 4, 2024 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔎 Awaiting review
Development

Successfully merging this pull request may close these issues.

2 participants