P2P: improve RX/TX flow control #2017
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
November 15, 2012 18:04
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.
| if (lockSend && !pnode->vSend.empty()) | ||
| FD_SET(pnode->hSocket, &fdsetSend); | ||
| if (lockSend) { | ||
| // do not read, if draining write queue |
There was a problem hiding this comment.
I don't understand this part. Why make fdsetRecv depend on the state of the write queue?
There was a problem hiding this comment.
@laanwj That is explained in the commit message / pull request message.
There was a problem hiding this comment.
But future readers of the code won't see your commit message. They need comments to understand what's going on.
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4e4e6d1feca92c346389d4221d022db8f8cd3e78 for binaries and test log. |
|
@jgarzik @gavinandresen I'd like to see this in 0.8.1. |
|
Superceded by #2409 |
KolbyML
pushed a commit
to KolbyML/bitcoin
that referenced
this pull request
Dec 5, 2020
1c43a1e GUI: Do not try to create a shielded address or process a shielded spend if v5 upgrade hasn't been enforced yet. (furszy) Pull request description: Visual notification, do not try to create shielded addresses or process shielded spends if v5 upgrade hasn't been enforced yet. ACKs for top commit: random-zebra: utACK 1c43a1e Fuzzbawls: ACK 1c43a1e Tree-SHA512: 22bbbc1cb3c485c67ba204c59421cbf4970ebb97d50bb25feabb5e6e77697ee2de93837982d345ef698ba4663142cdda306d2988eefd8c5c3eb0e363beb09ae8
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.