Network optimalizations #2409
Network optimalizations #2409
Conversation
|
ACK |
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/3987a1cabbdd263dfa7bed92b7326274bc625d09 for binaries and test log. |
|
Don't merge this yet; an instance of this segfaulted for me. |
|
Ok, fixed an obscure bug: ProcessMessages (while holding cs_vRecvMsg) sometimes calls PushMessage, which since Jeff's patches sometimes calls SocketSendData, which can fail and cause the connection to be closed. Closing the connection however wipes vRecvMsg... while ProcessMessages is still iterating over it, invalidating its iterator, and eventually trying to remove the entries it already processed, causing a double free. Seems to be running smoothly now. |
…hread Replaces CNode::vRecv buffer with a vector of CNetMessage's. This simplifies ProcessMessages() and eliminates several redundant data copies. Overview: * socket thread now parses incoming message datastream into header/data components, as encapsulated by CNetMessage * socket thread adds each CNetMessage to a vector inside CNode * message thread (ProcessMessages) iterates through CNode's CNetMessage vector Message parsing is made more strict: * Socket is disconnected, if message larger than MAX_SIZE or if CMessageHeader deserialization fails (latter is impossible?). Previously, code would simply eat garbage data all day long. * Socket is disconnected, if we fail to find pchMessageStart. We do not search through garbage, to find pchMessageStart. Each message must begin precisely after the last message ends. ProcessMessages() always processes a complete message, and is more efficient: * buffer is always precisely sized, using CDataStream::resize(), rather than progressively sized in 64k chunks. More efficient for large messages like "block". * whole-buffer memory copy eliminated (vRecv -> vMsg) * other buffer-shifting memory copies eliminated (vRecv.insert, vRecv.erase)
1) "optimistic write": Push each message to kernel socket buffer immediately. 2) If there is write data at select time, that implies send() blocked during optimistic write. Drain write queue, before receiving any more messages. This avoids needlessly queueing received data, if the remote peer is not themselves receiving data. Result: write buffer (and thus memory usage) is kept small, DoS potential is slightly lower, and TCP flow control signalling is properly utilized. The kernel will queue data into the socket buffer, then signal the remote peer to stop sending data, until we resume reading again.
* Change CNode::vRecvMsg to be a deque instead of a vector (less copying) * Make sure to acquire cs_vRecvMsg in CNode::CloseSocketDisconnect (as it may be called without that lock).
|
Added a commit, which breaks up the processing of getdata requests when the send buffer size limit is exceeded. This results in far smaller send buffers being allocated for peers downloading blocks from us, and doesn't seem to have a noticable impact on performance/bandwidth (I can still download blocks from my VPS running this, at ~6 MiB/s, same as before). |
|
Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/e96ba341587ebdb0583ce3cd0567411164f1ec44 for binaries and test log. This could happen for one of several reasons:
If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here. This is an automated test script which runs test cases on each commit every time is updated. |
|
@TheBlueMatt can you put a link to the patches applied for pulltester in the produced message? |
There exists a per-message-processed send buffer overflow protection, where processing is halted when the send buffer is larger than the allowed maximum. This protection does not apply to individual items, however, and getdata has the potential for causing large amounts of data to be sent. In case several hundreds of blocks are requested in one getdata, the send buffer can easily grow 50 megabytes above the send buffer limit. This commit breaks up the processing of getdata requests, remembering them inside a CNode when too many are requested at once.
|
Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/c7f039b674b43b741f20bf7521eb8a68426f4275 for binaries and test log. This could happen for one of several reasons:
If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here. This is an automated test script which runs test cases on each commit every time is updated. |
|
@sipa Updated the message, it now links to http://jenkins.bluematt.me/pull-tester/files/patches/ |
|
I'm testing this+Matts patches now - 231 connections with only 193mb RSS. Pretty fantastic results. Before this my node was running out of memory in less than 24 hours. |
|
This plus other recent network pulls has been stable for many hours under high load (150-200 connections). |
|
Is there a 0.8.2 planned in the near future? What will be in, was that yet decided? |
Network optimalizations
This pull request contains:
This should improve network latency and memory usage.