zmq: mempool notifications #7753

laanwj · 2016-03-27T16:29:16Z

Add notifications when transactions enter or leave the mempool.

These can be enabled with zmqpubmempool:

mempooladded: a transaction was added to the mempool
mempoolremoved: a transaction was removed from the mempool

This allows third-party software to keep track of what is in the mempool in real time. For example a mempool monitor I'm working on:

It also makes it possible to keep statistics on why transactions disappear from the mempool.

Full documentation: https://github.com/bitcoin/bitcoin/blob/325afe68381d59f1b95a690927c4a8ea886ac791/doc/zmq.md#mempooladd

See individual commits for details.

Current issues:

Warning NotifyEntryAdded: WARNING: zmq send failed with code -1 spam in log if "-zmqpubmempool" is not enabled.

promag · 2016-03-27T21:52:50Z

I was planning to submit a pull request to change this kind of notifications. The current solution is not very elegant and doesn't scale. The command line arguments should be clean.

So if you don't mind I'll try to explain here the idea using this example:

Usage: -notify "mempool ! json ! zmqpub address=tcp://127.0.0.1:28332"

Here the notify argument creates one notification pipeline composed by one source of events (mempool), one filter element to transform the event in json and one sink of events (zmqpub), all connected with the operator !. There can be more than one notifier configured.

Is this design, zmq elements are concrete types of sinks. But we could have more, like log to file or pub to nsq queue.

We could have chain and wallet sources for instance. We could have filters to transform to other types.

WDYT?

dcousens · 2016-03-27T23:34:55Z

concept ACK

laanwj · 2016-03-28T07:49:45Z

@promag I'm all for making it more flexible, but what you're proposing sounds like the gstreamer syntax and that's IMO even harder to use than the current arguments. I've needed to do a few media pipelines in my PhD research but never managed to really master it.

Our basic idea with the ZMQ notifications is that one notification system exists. From there on everything (e.g. forward to the notification system of your choice) can be handled by external software. I don't want too much notification-related complexity in core.

Aside, this is the wrong place to discuss this. Let's focus on the software change here. Please create a new issue.

promag · 2016-03-28T08:09:29Z

@laanwj yes gstreamer all over the comment. I'll move the discussion to a new issue.

utACK.

promag · 2016-03-28T08:10:56Z

src/init.cpp


 #if ENABLE_ZMQ
-    pzmqNotificationInterface = CZMQNotificationInterface::CreateWithArguments(mapArgs);
+    pzmqNotificationInterface = CZMQNotificationInterface::CreateWithArguments(mapArgs, &mempool);


Feels a bit hacky.

Better suggestions?
As I see ti there are two alternatives:

use the global mempool object (ugly)

pass the mempool in (slightly les ugly)

Another option would be to move the instantiation of the CZMQPublishMempoolNotifier(mempool) here. But I'd like as little zmq specific code in init. But it may be the least ugly option.

I think this implementation approach is reasonable and I also don't see a more elegant way to do this.

Then I would rename CreateWithArguments to just Create and move the following from src/init.cpp to Create.

if (pzmqNotificationInterface) { RegisterValidationInterface(pzmqNotificationInterface); }

Edit:
Because in your notifier you connect the signals so it also makes sense to connect the remaining signals here.

I agree, will move that line.

jonasschnelli · 2016-03-28T18:31:21Z

Awesome!
Nice work! I hope you have also plans to opensource the ncurses UI.

Some ideas / conceptual questions:

How could a listener keep track of the overall mempool size without listening to a fresh start (listening from the beginning when the mempool was at a size of 0), also there is a risk of a delta in case you missed a notification in case you calculate the mempool overall size on the "listeners side".
would a notification after a significant (every X MB, every X transaction added/removed) in the mempool size or structure make sense?
Adding the total mempool size in every mempool notification is probably to much?

jonasschnelli · 2016-03-28T18:34:37Z

Code Review utACK.

promag · 2016-03-28T19:14:27Z

Adding the total mempool size in every mempool notification is probably to much?

👍

laanwj · 2016-03-29T07:53:41Z

How could a listener keep track of the overall mempool size without listening to a fresh start (listening from the beginning when the mempool was at a size of 0), also there is a risk of a delta in case you missed a notification in case you calculate the mempool overall size on the "listeners side".

What if you use the following sequence:

Start listening (queue up events)
Request getrawmempool
Start applying events

You may have some duplicates in the first run, e.g., adds which already show up in the mempool requested, removes which were already removed, but I think due to the idempotent nature of the events (just add and remove) the overall state will be the same?

there is a risk of a delta in case you missed a notification

Missing notifications are a possibiltiy with zmq. But I'd say this is a matter of sizing buffers appropriately, and dedicating a thread to listening to zmq requests.

To reliably detect missing notifications we could add sequence numbers to the events, as I think zmq guarantees that events from a single source are delivered in order.

Then again if we do this we probably want it for all the event types... I actually suggested it for the first zmq proposal.

Adding the total mempool size in every mempool notification is probably to much?

I thought about sending a generic stats packet with every mempool event, but as you can receive tons of them I don't want to make the individual events too big and include things you can compute yourself. But no problems with just the size / # transactions.

Nice work! I hope you have also plans to opensource the ncurses UI.

Absolutely, I'll open source it soon, will be part of a bc-monitor repo. Eventually I hope to make a tool like tor-arm (just renamed to nyx IIRC) which allows watching all aspects of a node.

jonasschnelli · 2016-03-29T13:03:10Z

Tested ACK 4e3039b
I also think the addNotifier()-refactor makes it much cleaner.

paveljanik · 2016-03-30T14:20:22Z

lightly tested ACK 4e3039b

sipa · 2016-04-05T10:56:37Z

src/txmempool.h

+/** Reason why a transaction was removed from the mempool,
+ * this is passed to the notification signal.
+ */
+enum MemPoolRemovalReason {


This enum seems to be part of the ZMQ API, so we'll probably want some stability across versions. Things like MPR_EXPIRY and MPR_SIZELIMIT seem quite specific to the existing implementation.

laanwj · 2016-04-06T13:38:01Z

Ok added two commits:

The first adds a (currently, one to one) mapping between the mempool reasons from the mempool and those on the ZMQ interface.
The second is a slight refactoring to make all ZMQ publish notifiers register (and unregister) themselves for the events that they want to receive, instead of relying on the events being propagated manually though CZMQNotificationInterface @promag

laanwj · 2016-04-19T14:07:57Z

Rebased after #7762

Add notification signals to make it possible to subscribe to mempool changes: - NotifyEntryAdded(const CTxMemPoolEntry &)> - NotifyEntryRemoved(const CTxMemPoolEntry &, MemPoolRemovalReason)> Also add a mempool removal reason enumeration, which is passed to the removed notification based on why the transaction was removed from the mempool.

Replace factories list with calls to a function. This simplifies the code (every notifier is only created once anyway), and makes it possible to pass arguments to notifier contructors.

Add notifications when transactions enter or leave the mempool. These can be enabled with `pubmempool`: - `mempooladded`: a transaction was added to the mempool - `mempoolremoved`: a transaction was removed from the mempool This allows third-party software to keep track of what is in the mempool in real time.

Make the registration process less circuitous by making the notifiers listen to their appropriate events themselves

laanwj · 2016-05-12T10:16:12Z

Rebased, and changed the enums (both for the mempool and for the zmq protocol) to c++11 enums.

paveljanik · 2016-05-12T15:04:02Z

lightly tested ACK 1bbe366

sipa · 2016-05-25T16:20:57Z

Needs rebase.

jonasschnelli · 2016-06-10T09:15:32Z

Still needs rebase.

jmcorgan · 2016-08-19T01:50:52Z

ACK. I have a rebased version of this branch at 'jmcorgan/2016_03_zmq_mempool_notifications'.

jonasschnelli · 2016-08-19T09:09:01Z

@jmcorgan feel free to open a PR of your rebases version and take over the lead.

jmcorgan · 2016-08-19T17:08:55Z

Continued in #8549

laanwj · 2016-10-18T13:45:58Z

Closing in favor of #8549

laanwj added the RPC/REST/ZMQ label Mar 27, 2016

laanwj force-pushed the 2016_03_zmq_mempool_notifications branch from 325afe6 to 4e3039b Compare March 27, 2016 18:17

promag reviewed Mar 28, 2016
View reviewed changes

laanwj force-pushed the 2016_03_zmq_mempool_notifications branch from 00b0a36 to 4e3039b Compare March 29, 2016 11:43

sipa reviewed Apr 5, 2016
View reviewed changes

laanwj force-pushed the 2016_03_zmq_mempool_notifications branch from bde966e to 85cbe30 Compare April 19, 2016 14:07

laanwj added 5 commits May 12, 2016 12:15

zmq: refactor initialization

dad857f

Replace factories list with calls to a function. This simplifies the code (every notifier is only created once anyway), and makes it possible to pass arguments to notifier contructors.

doc: Add documentation for zmq mempool notifications

2bb2e40

zmq: refactor to make all notifiers register themselves

1bbe366

Make the registration process less circuitous by making the notifiers listen to their appropriate events themselves

laanwj force-pushed the 2016_03_zmq_mempool_notifications branch from 85cbe30 to 1bbe366 Compare May 12, 2016 10:15

laanwj added this to the Future milestone Jun 13, 2016

jmcorgan mentioned this pull request Aug 19, 2016

zmq: mempool notifications #8549

Closed

laanwj closed this Oct 18, 2016

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

maflcko removed this from the Future milestone Jul 23, 2025

zmq: mempool notifications #7753

zmq: mempool notifications #7753

Uh oh!

Conversation

laanwj commented Mar 27, 2016

Uh oh!

promag commented Mar 27, 2016

Uh oh!

dcousens commented Mar 27, 2016

Uh oh!

laanwj commented Mar 28, 2016

Uh oh!

promag commented Mar 28, 2016

Uh oh!

promag Mar 28, 2016

Choose a reason for hiding this comment

Uh oh!

laanwj Mar 28, 2016

Choose a reason for hiding this comment

Uh oh!

jonasschnelli Mar 28, 2016

Choose a reason for hiding this comment

Uh oh!

promag Mar 28, 2016

Choose a reason for hiding this comment

Uh oh!

laanwj Apr 5, 2016

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Mar 28, 2016

Uh oh!

jonasschnelli commented Mar 28, 2016

Uh oh!

promag commented Mar 28, 2016

Uh oh!

laanwj commented Mar 29, 2016

Uh oh!

jonasschnelli commented Mar 29, 2016

Uh oh!

paveljanik commented Mar 30, 2016

Uh oh!

sipa Apr 5, 2016

Choose a reason for hiding this comment

Uh oh!

laanwj commented Apr 6, 2016

Uh oh!

laanwj commented Apr 19, 2016

Uh oh!

laanwj commented May 12, 2016

Uh oh!

paveljanik commented May 12, 2016

Uh oh!

sipa commented May 25, 2016

Uh oh!

jonasschnelli commented Jun 10, 2016

Uh oh!

jmcorgan commented Aug 19, 2016

Uh oh!

jonasschnelli commented Aug 19, 2016

Uh oh!

jmcorgan commented Aug 19, 2016

Uh oh!

laanwj commented Oct 18, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants