Parallel ThreadMessageHandler

TheBlueMatt commented at 3:35 am on January 8, 2017: member

Based on a (now outdated) #9441.

This runs multiple ThreadMessageHandlers, but only allows them to do (relatively limited) work - it has a whitelisted list of commands which are expected to not take cs_main, and only runs those in a secondary thread, but running anything in the “main thread” (a concept based on randomly acquiring an atomic_bool at the top of the processing loop).

Additionally, it will never be in the ProcessMessages/SendMessages part of the loop for one node in both threads.

With this, #9419, and #9375 (plus changing the whitelisted list of messages) we can respond to getblocktxn requests while another ProcessMessages is busy connecting the block.

net: fix typo causing the wrong receive buffer size

Surprisingly this hasn't been causing me any issues while testing, probably
because it requires lots of large blocks to be flying around.

Send/Recv corks need tests!

53ad9a133a

net: make vRecvMsg a list so that we can use splice() e5bcd9c84f

net: make GetReceiveFloodSize public

This will be needed so that the message processor can cork incoming messages

5b4a8ac6d6

net: only disconnect if fDisconnect has been set

These conditions are problematic to check without locking, and we shouldn't be
relying on the refcount to disconnect.

f6315e07f9

net: wait until the node is destroyed to delete its recv buffer

when vRecvMsg becomes a private buffer, it won't make sense to allow other
threads to mess with it anymore.

60425870d7

net: remove redundant max sendbuffer size check

This is left-over from before there was proper accounting. Hitting 2x the
sendbuffer size should not be possible.

0e973d970a

net: set message deserialization version when it's actually time to deserialize

We'll soon no longer have access to vRecvMsg, and this is more intuitive anyway.

56212e20ac

net: handle message accounting in ReceiveMsgBytes

This allows locking to be pushed down to only where it's needed

Also reuse the current time rather than checking multiple times.

cb3456eb3c

net: record bytes written before notifying the message processor b8f8d61a65

net: Add a simple function for waking the message handler

This may be used publicly in the future

3d23d9fe6b

net: remove useless comments 13234c25af

net: rework the way that the messagehandler sleeps

In order to sleep accurately, the message handler needs to know if _any_ node
has more processing that it should do before the entire thread sleeps.

Rather than returning a value that represents whether ProcessMessages
encountered a message that should trigger a disconnnect, interpret the return
value as whether or not that node has more work to do.

Also, use a global fProcessSleep value that can be set by other threads,
which takes precedence (for one cycle) over the messagehandler's decision.

Note that the previous behavior was to only process one message per loop
(except in the case of a bad checksum or invalid header). That was added in

The only change here in that regard is that the current node now falls to the
back of the processing queue for the bad checksum/invalid header cases.

4157c99cd8

net: add a new message queue for the message processor

This separates the storage of messages from the net and queued messages for
processing, allowing the locks to be split.

b5ea4f2199

net: add a flag to indicate when a node's process queue is full

Messages are dumped very quickly from the socket handler to the processor, so
it's the depth of the processing queue that's interesting.

The socket handler checks the process queue's size during the brief message
hand-off and pauses if necessary, and the processor possibly unpauses each time
a message is popped off of its queue.

a109f7bd18

net: add a flag to indicate when a node's send buffer is full

Similar to the recv flag, but this one indicates whether or not the net's send
buffer is full.

The socket handler checks the send queue when a new message is added and pauses
if necessary, and possibly unpauses after each message is drained from its buffer.

e047bde489

net: split the message queueing into its own function

This is representative of how messages will be sent out once processing is
abstracted out. Makes it clear that the processor _must_ accept all messages.

It also pushes the use of cs_vProcessMsg to a function with narrow scope.

90bfeb0c2e

net: drop the receive lock during message processing

It's now only used for message size/time accounting

1c779a8cfe

Add a new lock to CNode to process one node at once cf35386606

fanquake added the label P2P on Jan 8, 2017

in src/net_processing.cpp: in c4d48af310 outdated

2469@@ -2470,6 +2470,19 @@ unsigned int ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic<bool>&
2470             LOCK(pfrom->cs_vProcessMsg);
2471             if (pfrom->vProcessMsg.empty())
2472                 return 0;
2473+
2474+            // Check if we are supposed to avoid cs_main, but are about to try to lock it

gmaxwell commented at 5:29 am on January 8, 2017:

cs_main is taken by ProcessGetData at like 2456 above. (github won’t let me comment there).

TheBlueMatt commented at 5:49 am on January 8, 2017:

Fixed

gmaxwell commented at 5:35 am on January 8, 2017: contributor

I am somewhat dubious that it’s feasible to determine and maintain that various bits of code do not take cs_main anywhere in their call graph.

I think code like this might need a new debug tool where you can poison_lock(cs_main) which increments a thread local counter while it is in scope. And if the counter is positive when cs_main is taken it causes an assertion/debug print.

TheBlueMatt force-pushed on Jan 8, 2017

TheBlueMatt commented at 5:45 am on January 8, 2017: member

Yea, I thought about adding something like that and never got around to it…I’ll try to add it to this in the coming days.

Make ProcessMessages return whether the next message needs cs_main c4b6c9144e

Give ProcessMessages a boolean to tell it to avoid cs_main locking 6b4a883708

Make ThreadMessageHandler support running in parallel

This uses the new return value from ProcessMessages which indicates
whether the next message might not need cs_main, trying to process
any messages which do not need cs_main, even while another thread
is running which takes cs_main for messages.

4632608407

Run multiple (default 2) ProcessMessages threads ac0a3ad90b

TheBlueMatt force-pushed on Jan 8, 2017

Add the ability to disallow a lock in DEBUG_LOCKORDER 93bbd0a2dc

Disallow cs_main in ProcessMessages if we have fAvoidLocking 2e036fe060

TheBlueMatt commented at 7:57 pm on January 10, 2017: member

Added a DEBUG_LOCKORDER check to prevent cs_main.

TheBlueMatt commented at 3:42 pm on January 17, 2017: member

Closing for now. Will work towards a more whole solution for 0.15.

TheBlueMatt closed this on Jan 17, 2017

MarcoFalke locked this on Sep 8, 2021

Parallel ThreadMessageHandler #9488