-
Notifications
You must be signed in to change notification settings - Fork 127
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
Introduce Zmq and interrupt the main loop wait if zmq is setup and a block hash notification is received #111
base: new-index
Are you sure you want to change the base?
Conversation
73a70a4
to
bfb2985
Compare
There's an WIP implementation of this I wrote awhile ago in this branch. It has basic working functionality for updating the mempool view with individual transactions as they arrive from zmq rather than doing full periodic updates, but is still incomplete and missing some important optimizations (in particular for updating the mempool scripthash history indexes, which was already inefficient but is in a much hotter code patch with this change).
This can be independent of a tx cache if you patch the mempool view directly with new transactions, instead of keeping them around but only applying them on the next periodic update. My understanding is that in the current blockstream.info setup, there's a ZMQ client running alongside electrs that receives Given this, is there much of an advantage to moving the I do see a significant advantage to using zmq for updating the mempool with individual transactions, both for performance and to keep the electrs mempool view more closely in-sync with bitcoind's, without the latency introduced by periodic updates. However if that's our goal, perhaps we should aim straight for an MR that includes it too? I'm not sure that integrating zmq is worth it just for block updates, and it might be beneficial to design the zmq integration with mempool updates in mind from the get go. |
Yes, so we can get rid of a service in the infra written in go that listen to zmq and send signals, it simplify the infra to have this logic in electrs instead.
We would need zmq thread similar to this for it? This seems a step in that direction and also useful to get rid of a part of the infra (the service with the purpose of sending signals). But I am fine also if we want to go with the branch instead (adding also the wakeup in the loop) if it can be done in a reasonable timeframe. |
Right, we do. If we do just blocks subscription, then there are some things I slightly prefer in the branch (using an optional feature flag and the ZmqSyncer encapsulation) but none are critical and we could go with this PR too.
In the branch there no longer is a sync loop when ZMQ is enabled, it relies on the subscriptions instead of doing periodic updates:
Well define reasonable :) ZMQ-based mempool syncing is quite more involved than just block subscription so it would take longer to get right and require more throughout testing. The upside is big and worth it imo, but if doing just block subscription to get rid of the zmq service is useful on its own then I wouldn't hold it up for that. |
In a follow up MR we can also listen to transactions from zmq so that the mempool update already finds transactions and doesn't need to fetch most of them from the node (probably needs a tx cache #110 )