-
Notifications
You must be signed in to change notification settings - Fork 723
[WIP] make event dispatching non-blocking #6762
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
base: develop
Are you sure you want to change the base?
[WIP] make event dispatching non-blocking #6762
Conversation
The observer is now a pure data struct without any methods. All functionality lives in the dispatcher; the observer just contains the info where to send the data. This is a largely mechanical change: cut & paste; delete now duplicate `delete_payload` method; fix references.
No functional changes, only moving things around
There should be no functional change here.
| @@ -0,0 +1,322 @@ | |||
| use std::path::PathBuf; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Please add the copyright header to each source file. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good call, will do. Is "Stacks Open Internet Foundation" still the correct copyright holder?
Also, there's a whole bunch of files that lack that header, I'm wondering if there's a good way to automate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the code in new files is just cut & paste from `event_dispatcher.rs`, I kept the same header from there, but bumped the year to 2025.
This is the final step to making `EventObserver` a "dumb" data object that simply reflects the configuration of the registered observer. Most of this change is straightforward. There is one thing that's worth a mention, and that's the `ProposalCallbackHandler`, a concept that was added in stacks-network#4228. Because block proposal validation happens on a dedicated thread, those validation events are handled a little specially. Since `send_payload` now needs a dispatcher instance as the source of the `db_path`, the list of observers alone is no longer enough. For now I've elected to simply add the DB path to the ProposalCallbackHandler and keep a non-method version of `send_payload` around that gets the DB path passed in. Eventually I may have to clone the dispatcher, but for now this approach had the smallest blast radius. There wasn't any test coverage for this special behavior (at least not on the event dispatcher side -- there are [some tests](https://github.com/stacks-network/stacks-core/blob/45381558f5e79554fc6e7930b0c4091e739e4992/stackslib/src/net/api/tests/postblock_proposal.rs#L234) on the API side of things), so I added that as well.
Codecov Report❌ Patch coverage is ❌ Your project check has failed because the head coverage (67.73%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.
Additional details and impacted files@@ Coverage Diff @@
## develop #6762 +/- ##
============================================
- Coverage 78.18% 67.73% -10.46%
============================================
Files 580 584 +4
Lines 361096 361345 +249
============================================
- Hits 282312 244741 -37571
- Misses 78784 116604 +37820
... and 316 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Description
Applicable issues
Additional info (benefits, drawbacks, caveats)
Checklist
docs/rpc/openapi.yamlandrpc-endpoints.mdfor v2 endpoints,event-dispatcher.mdfor new events)clarity-benchmarkingrepo