High HTTP traffic nodes are disconnected from peers as unresponsive

[kantai]

HTTP RPC traffic in stacks-blockchain nodes can prevent the nodes from synchronizing with other peers due to "unresponsiveness". This isn't problematic for non-public nodes, but if a node is intended to service RPC traffic, for example the node is a public node backing an API service, this can be extremely problematic. As an HTTP backlog forms on the node, other nodes will disconnect it:

Disconnect unresponsive unauthenticated peer convo:id=667,outbound=true,peer=UNKNOWN+UNKNOWN://...:20444: 1625777051 + 5 < 1625777057

The node is then unable to make any progress and falls behind the network.

I think there's a couple of possible ways to resolve this:

1. Prioritization of request handling. It seems like the overwhelmed node isn't responding to it's own initialized handshake. Perhaps those kinds of requests should be bumped up in priority?
2. Add an allow list configuration option. A setup with public nodes that expects to occasionally experience near-capacity request throughput could have non-public nodes that sync with the public nodes. These non-public nodes could be configurable such that they never disconnect the public nodes.

[jcnelson]

I think an easy win here could be to simply bound the amount of time we're allowed to spend on http traffic. We could randomize the order in which we process ready http sockets, and set a deadline on ready http socket-processing -- if more than $QUANTUM milliseconds pass, then break out of the socket-processing loop. The randomized iteration ensures http liveness (albeit with longer latencies and lower bandwidth under load), and the processing deadline ensures that we never go more than $QUANTUM milliseconds between polling the p2p port (preventing p2p starvation).

More generally, we need to revisit a few things about I/O-heavy http requests like running read-only functions. Do you have any data on what the http load looked like when you noticed that p2p peers started getting disconnected?

[jcnelson]

Providing a little bit more background

• HTTP requests against Clarity state are all read-only, and could be parallelized. However, this is difficult for two reasons:
  • There can only one instance of the unconfirmed Clarity state (since re-calculating it on-the-fly is prohibitively expensive), and it can be dropped at any time by the relayer thread when a Stacks block is processed that confirms it. Safe access to the unconfirmed state trie is currently shared with the p2p thread via sqlite via an internal read/write lock -- the unconfirmed state trie cannot be dropped while the HTTP server is processing a Clarity request on it. Safe access to the set of unconfirmed transactions is currently handled by copying the set of txids from the relayer thread's StacksChainState to the p2p thread's StacksChainState (not the most elegant way to handle this, but the least disruptive way to handle this at the time it was implemented).
  • The act of processing a read-only transaction in Clarity requires a mutable reference to the underlying MARF, since the Clarity VM must instantiate a new MARF trie before any VM processing can take place (but note that this does not do any I/O). The VM receives a mutable reference to the MARF held by the StacksChainState owned by the p2p network thread.
• HTTP requests to store transactions are necessarily serialized against one another, since they all carry out transactions against the mempool database. However, they can run in parallel to all other requests.
• HTTP requests to store blocks and microblocks are necessarily serialized against one another, since they all carry out writes to the StacksChainState database (i.e. they both preprocess blocks and microblocks, which writes them to the staging tables). However, they can run in parallel to all other requests.

If we're going to try and safely parallelize HTTP requests, I think we should create a set of API handles to the StacksChainState that make the following static guarantees:

• Read-only access to the mempool
• Read-only access to the staging blocks and microblocks tables and chunk store
• Read-write access to the mempool
• Read-write access to the staging blocks and microblocks tables
• Read-only access to the Clarity state, including a read-only reference to a shared singleton unconfirmed Clarity state

All of this is certainly doable (they're really just wrappers around a StacksChainState), and in all but the case of the unconfirmed transaction set, safe read/write access is enforced at runtime via sqlite.

[jcnelson]

I'm about 2/3 of the way through a PR that adds the safe StacksChainState interfaces. Once they're ready, I'll send it over -- it will pave the way towards safely handling HTTP requests in a threadpool.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed. Please reopen if needed.

[blockstack-devops]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.