-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
feat: log poller for vrf v2/v2+ #11174
Conversation
I see that you haven't updated any README files. Would it make sense to do so? |
cf415bd
to
654c261
Compare
After running a short load test it seems that this approach of having everything in a single loop has actually caused a regression in performance on the simulated networks we use for load tests. I'm not sure of what is causing it, perhaps the log poller interval is too high, and perhaps the VRF job interval is also too high. If so I'll set them appropriately and run the test again. |
Update: after bumping log poll interval down to 1s the load tests are looking on-par with the log-broadcaster-based VRF listener in the simulated environment. |
1ee78a6
to
d6e83b7
Compare
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.
Still reviewing! Mostly looked at the new main LP logic
these tests have questionable value; we should probably remove them or find a better way to test this particular functionality. Perhaps this should even be removed.
seems unnecessary
* feat: log poller for vrf v2/+ * chore: address PR comments * fix: RLock() instead of Lock() * fix: test * fix: tests still flakey * fix: lint * fix: build * chore: PR comments * chore: time out old requests * chore: skip EOA tests these tests have questionable value; we should probably remove them or find a better way to test this particular functionality. Perhaps this should even be removed. * remove concurrency in processing seems unnecessary * fix build * fix build * fix lint * fix test * fix tests
Switch over the VRF V2 listener to use the log poller instead of the log broadcaster.
Summary of Changes
runLogListener
inlistener_v2_log_listener.go
for the gory details):The effect of this is that the processing loop of the VRF listener is much simpler, concurrent access to
lsn.reqs
is no longer needed, and in fact thereqs
in-memory queue can be completely removed, as well as any associated data structures to protect it from data races. The functionrunRequestHandler
has also been removed, since pending request processing is in the same loop as the log polling.a.
listener_v2_log_listener.go
: this file contains all log polling and listening logic, as well as the main looprunLogListener
. This is the entrypoint into the functionality of the VRF listener. This file contains the lion's share of new code in this PR, mainly the revampedrunLogListener
, and the newinitializeLastProcessedBlock
,updateLastProcessedBlock
, andpollLogs
methods.b.
listener_v2_log_processor.go
: this file contains all log processing logic, the entrypoint of which isprocessPendingVRFRequests
. Notably,processPendingVRFRequests
marks all in-flight fulfillments in theinflightCache
cache, so that these requests don't get reprocessed. Most of the code in this file is unchanged and was simply copied over fromlistener_v2.go
.c.
listener_v2_helpers.go
: contains helper functions and types, extracted fromlistener_v2.go
.