rpc: Add mempoolchanges

promag commented at 6:46 pm on July 9, 2020: member

The new RPC mempoolchanges allows a client to retrieve mempool changes in a reliable way.

The client needs to start a dedicated stream:

0bitcoin-cli mempoolchanges start
11

This returns the stream ID and at this point it contains the mempool content and it will keep track of all mempool changes.

To retrieve the actual changes the client calls:

0bitcoin-cli mempoolchanges pull 1 10
1[
2]

This pulls up to 10 changes from the given stream.

Finally the client can stop the stream with

0bitcoin-cli mempoolchanges stop 1

Also, the stream is limited and if that limit is reached then the stream is automatically stopped.

TODO: option to block until changes are available.

promag commented at 6:47 pm on July 9, 2020: member

This is a POC, please don’t review the code, just the approach. It needs to define the change format among other details.

luke-jr commented at 7:33 pm on July 9, 2020: member

Seems like this should probably use longpolling instead?

promag commented at 7:46 pm on July 9, 2020: member

Pulling can also block until changes are available. @luke-jr ☝️

instagibbs commented at 7:47 pm on July 9, 2020: member

Might be a good idea to optionally start with full getrawmempool results(if it’s not already)

promag commented at 7:49 pm on July 9, 2020: member

@instagibbs that’s already the case, start fills the stream with add <hash> entries (don’t mind the format) so a client never needs to call getrawmempool and compare/diff and sync with zmq notifications.

luke-jr commented at 7:57 pm on July 9, 2020: member

Longpolling would support multiple clients on a single buffer. So each request would get a new position id, and requesting with that position id would tell the node where in the shared buffer to begin with new data.

promag commented at 8:08 pm on July 9, 2020: member

@luke-jr yes, it would be possible to share the buffer for all active clients. The problem is when a new client starts a stream, it needs to receive the current mempool and then resume from the shared buffer. I consider this an optimization.

DrahtBot added the label RPC/REST/ZMQ on Jul 9, 2020

promag renamed this:
~~wip, rpc: Add mempoolchanges~~
rpc: Add mempoolchanges
on Jul 12, 2020

DrahtBot commented at 6:04 am on July 13, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#19849 (Assert that RPCArg names are equal to CRPCCommand ones (blockchain,rawtransaction) by MarcoFalke)
#19725 ([RPC] Add connection type to getpeerinfo, improve logs by amitiuttarwar)
#19572 (ZMQ: Create “sequence” notifier, enabling client-side mempool tracking by instagibbs)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

promag force-pushed on Jul 15, 2020

rpc: Add mempoolchanges 353efdc48f

promag force-pushed on Jul 15, 2020

promag marked this as ready for review on Jul 15, 2020

qa: Test mempoolchanges RPC 47083b0b4c

promag force-pushed on Jul 16, 2020

instagibbs commented at 8:31 pm on July 23, 2020: member

Anyone interested in this may be interested in #19572 as well.

shuckc commented at 9:32 am on July 24, 2020: none

I promised a quick review after the developer IRC chat. My concerns would be:

If a client is struggling to handle a particular message type, it’s not unimaginable that it might keep re-connecting for a new stream, getting a message and failing. Each time the MempoolChanges tracker gets abandoned by the client, but keeps accumulating new events until it hits the backlog-limit. The side-effects of not being able to tie the event stream to a particular client’s auto-closable resource (ie. a particular TCP session) are wide ranging. It’s likely we’d have to limit the number of currently open streams, and stop serving new mempoolchanges start requests, to avoid death by malloc fails. As we don’t know how long it will take until the old streams die from overflowing (max depth/current event rate) what would be a sensible retry for a client in this situation? You can’t guarantee evicting the oldest stream would help, as that might be a good consumer who is keeping up. So perhaps you’d have to walk the list of open sessions and evict the ‘most-backlogged’ when a new stream is requested and we are over limit. Which in itself is quite a bit of code to do and will always trigger with worst-case full lists to walk. Many magic parameters to tune.

Since there’s no TCP session for each resource, how would I troubleshoot a bitcoind that’s no longer responding to mempoolchanges start? With ZMQ there is a kernel max-backlog depth for accepting sockets, and I can use netstat or my OS tools of choice to find the culprit who is hogging the resources. With these anonymous sessions, I’d need some sort of a debug command in bitcoind to show me all the open MempoolChanges instances, with the last connecting endpoint’s IP address/hostname, and the backlog for each for it to be of any use. An administrators nightmare.

There is also notably no isolation between streams. I can cause havoc (or at least, defeat the whole reliable nature of this scheme) by requesting an event from a different client’s stream. The original client is then gaped and has no idea. You could defend against this by including the stream sequence number in the reply (so the client can detect gaps) and issuing the stream identifier numbers secure-randomly (to make them non-predictable), but these are both extra bytes you need to carry around on every request/response that a per-subscriber TCP stream does away with.

promag commented at 10:16 am on July 24, 2020: member

Thanks for the comments @shuckc

I’m not that worried about memory. Of course it would be a problem if you start lots of streams and you don’t poll from them. We could limit the sum of all streams sizes. Anyway, internally streams could share the same data, see #19476 (comment).

Currently the client has to periodically request for changes, but the connection can be reused as we support multiple requests on the same connection. Long polling is not implemented, but once it’s implemented it only reduces unnecessary empty requests.

I think this approach would be preferable in the REST interface with actual long polling, where the response doesn’t actually finishes, but the client keeps reading them. But we don’t have support in http server for this yet.

With these anonymous sessions, I’d need some sort of a debug command in bitcoind to show me all the open MempoolChanges instances, with the last connecting endpoint’s IP address/hostname, and the backlog for each for it to be of any use.

I thought of mempoolchanges status where it would return the list and stream size.

There is also notably no isolation between streams. I can cause havoc (or at least, defeat the whole reliable nature of this scheme) by requesting an event from a different client’s stream. The original client is then gaped and has no idea.

I’m assuming clients don’t mess around with other streams, that’s why the stream id isn’t reused. I though about letting the client give a name for the stream (start foo) but meh.

but these are both extra bytes you need to carry around on every request/response that a per-subscriber TCP stream does away with.

A per-subscriber TCP stream isn’t fault tolerant, that’s the point here where the server holds the changes until that subscriber comes again.

DrahtBot added the label Needs rebase on Sep 22, 2020

DrahtBot commented at 5:07 pm on September 22, 2020: member

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

promag commented at 5:32 pm on October 5, 2020: member

Closing now that #19572 is merged.

promag closed this on Oct 5, 2020

promag deleted the branch on Oct 5, 2020

DrahtBot locked this on Feb 15, 2022

rpc: Add mempoolchanges #19476

Conflicts