zmq: mempool notifications

laanwj commented at 4:29 pm on March 27, 2016: member

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: schermafdruk van 2016-03-27 17-42-16 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.

laanwj added the label RPC on Mar 27, 2016

laanwj force-pushed on Mar 27, 2016

promag commented at 9:52 pm on March 27, 2016: member

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 commented at 11:34 pm on March 27, 2016: contributor

concept ACK

laanwj commented at 7:49 am on March 28, 2016: member

@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 commented at 8:09 am on March 28, 2016: member

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

utACK.

in src/init.cpp: in 4e3039b419 outdated

1229@@ -1230,7 +1230,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
1230         AddOneShot(strDest);
1231 
1232 #if ENABLE_ZMQ
1233-    pzmqNotificationInterface = CZMQNotificationInterface::CreateWithArguments(mapArgs);
1234+    pzmqNotificationInterface = CZMQNotificationInterface::CreateWithArguments(mapArgs, &mempool);

promag commented at 8:10 am on March 28, 2016:

Feels a bit hacky.

laanwj commented at 9:29 am on March 28, 2016:

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.

jonasschnelli commented at 6:32 pm on March 28, 2016:

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

promag commented at 7:12 pm on March 28, 2016:

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

0if (pzmqNotificationInterface) {
1    RegisterValidationInterface(pzmqNotificationInterface);
2}

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

laanwj commented at 9:33 am on April 5, 2016:

I agree, will move that line.

jonasschnelli commented at 6:31 pm on March 28, 2016: contributor

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 commented at 6:34 pm on March 28, 2016: contributor

Code Review utACK.

promag commented at 7:14 pm on March 28, 2016: member

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

:+1:

laanwj commented at 7:53 am on March 29, 2016: member

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.

laanwj force-pushed on Mar 29, 2016

jonasschnelli commented at 1:03 pm on March 29, 2016: contributor

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

paveljanik commented at 2:20 pm on March 30, 2016: contributor

lightly tested ACK https://github.com/bitcoin/bitcoin/commit/4e3039b41946ad7bbb1aff6b6df5b1e83348c942

in src/txmempool.h: in 4e3039b419 outdated

320@@ -319,6 +321,18 @@ class CInPoint
321     size_t DynamicMemoryUsage() const { return 0; }
322 };
323 
324+/** Reason why a transaction was removed from the mempool,
325+ * this is passed to the notification signal.
326+ */
327+enum MemPoolRemovalReason {

sipa commented at 10:56 am on April 5, 2016:

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.

in src/zmq/zmqpublishnotifier.cpp: in 4e3039b419 outdated

207+void CZMQPublishMempoolNotifier::NotifyEntryRemoved(const CTxMemPoolEntry &entry, MemPoolRemovalReason reason)
208+{
209+    LogPrint("zmq", "zmq: mempool entry removed: %s, reason %i\n", entry.GetTx().GetHash().ToString(), (int)reason);
210+    uint8_t data[32 + 1];
211+    WriteHash(&data[0], entry.GetTx().GetHash());
212+    data[32] = reason;

sipa commented at 11:05 am on April 5, 2016:

I would prefer it if the value of the (internal) MemPoolRemovalReason did not leak into the ZMQ protocol, though that can be fixed later by introducing a new enum in the ZMQ definition.

laanwj commented at 11:07 am on April 5, 2016:

Yes, I’ll add another enum here and add a mapping.

laanwj commented at 1:38 pm on April 6, 2016: member

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 force-pushed on Apr 19, 2016

laanwj commented at 2:07 pm on April 19, 2016: member

Rebased after #7762

mempool: add notification for added/removed entries

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.

8b205dd6fa

zmq: refactor initialization

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.

dad857f8cd

zmq: add mempool notifications

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.

fc18de13c8

doc: Add documentation for zmq mempool notifications 2bb2e402d7

zmq: refactor to make all notifiers register themselves

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

1bbe3660ad

laanwj force-pushed on May 12, 2016

laanwj commented at 10:16 am on May 12, 2016: member

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

paveljanik commented at 3:04 pm on May 12, 2016: contributor

lightly tested ACK https://github.com/bitcoin/bitcoin/pull/7753/commits/1bbe3660ad9a375e2d2bb8c7fa8f38132774b656

sipa commented at 4:20 pm on May 25, 2016: member

Needs rebase.

jonasschnelli commented at 9:15 am on June 10, 2016: contributor

Still needs rebase.

laanwj added this to the milestone Future on Jun 13, 2016

jmcorgan commented at 1:50 am on August 19, 2016: contributor

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

jonasschnelli commented at 9:09 am on August 19, 2016: contributor

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

jmcorgan commented at 5:08 pm on August 19, 2016: contributor

Continued in #8549

laanwj commented at 1:45 pm on October 18, 2016: member

Closing in favor of #8549

laanwj closed this on Oct 18, 2016

DrahtBot locked this on Sep 8, 2021

zmq: mempool notifications #7753