Relay blocks as a "preview" before checking the transactions in them #1586

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@luke-jr
Member

luke-jr commented Jul 12, 2012

DRAFT status, please help test! Trying to coordinate testing on IRC, possibly using NTP and https://gist.github.com/3094657

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Aug 1, 2012

Contributor

@gmaxwell and I are definitely interested in this, at least. Will keep this pull request open.

A rebase would be nice, if you have time.

Contributor

jgarzik commented Aug 1, 2012

@gmaxwell and I are definitely interested in this, at least. Will keep this pull request open.

A rebase would be nice, if you have time.

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Aug 13, 2012

Member

Unfortunately, it seems p2p connections are handled synchronously, so everything this does to rush out the relaying is for naught. The only simple way I can see to refactor this to be asynchronous would be either to use coroutines (which Boost won't have until 2013) or one thread per command/connection. Either one is probably too late for 0.7, as would be an even larger refactor, I presume?

Member

luke-jr commented Aug 13, 2012

Unfortunately, it seems p2p connections are handled synchronously, so everything this does to rush out the relaying is for naught. The only simple way I can see to refactor this to be asynchronous would be either to use coroutines (which Boost won't have until 2013) or one thread per command/connection. Either one is probably too late for 0.7, as would be an even larger refactor, I presume?

@BitcoinPullTester

This comment has been minimized.

Show comment
Hide comment

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4e54ea804ccdd2223e622497f0d46cceb27b9d22 for binaries and test log.

@BitcoinPullTester

This comment has been minimized.

Show comment
Hide comment
@BitcoinPullTester

BitcoinPullTester Jul 20, 2013

Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/4e54ea804ccdd2223e622497f0d46cceb27b9d22 for test log.

This pull does not merge cleanly onto current master
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/4e54ea804ccdd2223e622497f0d46cceb27b9d22 for test log.

This pull does not merge cleanly onto current master
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Aug 25, 2013

Contributor

Closing - timeout. Interesting proposal, needs more work and dev momentum.

Contributor

jgarzik commented Aug 25, 2013

Closing - timeout. Interesting proposal, needs more work and dev momentum.

@jgarzik jgarzik closed this Aug 25, 2013

suprnurd pushed a commit to chaincoin-legacy/chaincoin that referenced this pull request Dec 5, 2017

Backport Bitcoin PR#9441: Net: Massive speedup. Net locks overhaul (#…
…1586)

* 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!

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

* net: make GetReceiveFloodSize public

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

* 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.

* 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.

* 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.

* 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.

* net: record bytes written before notifying the message processor

* net: Add a simple function for waking the message handler

This may be used publicly in the future

* net: remove useless comments

* 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.

* 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 fProcessWake 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 changed in
PR #3180.

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.

* 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.

* 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.

* 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.

* net: remove cs_vRecvMsg

vRecvMsg is now only touched by the socket handler thread.

The accounting vars (nRecvBytes/nLastRecv/mapRecvBytesPerMsgCmd) are also
only used by the socket handler thread, with the exception of queries from
rpc/gui. These accesses are not threadsafe, but they never were. This needs to
be addressed separately.

Also, update comment describing data flow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment