Stop processing messages on full send buffer and dont disconnect. #973

pull TheBlueMatt wants to merge 1 commits into bitcoin:master from TheBlueMatt:diffsendbuffer changing 4 files +14 −17
  1. TheBlueMatt commented at 2:39 am on March 22, 2012: member

    Also decrease default send/receive buffer sizes from 10 to 5 mb as this patch makes it easy for a node to fill both instead of only send.

    The largest advantage is not having to read from disk or serialize blocks on getblocks requests.

  2. TheBlueMatt commented at 2:42 am on March 22, 2012: member
    I did test downloading from block 143XXX to current tip and didnt see any issues (nor even any warnings about send buffer being full, which makes sense given that this is downloading from one local node to another)
  3. jgarzik commented at 4:03 am on March 22, 2012: contributor

    Looks like fun stuff to play around with… building now.

    Disagree with the minor AddAddress debug change. Once a node is running, these messages are (a) minimal and (b) useful. Better approach is to start separating messages at a much more fine grain, by introducing, e.g. fNetDebug (network-related messages) and fTxDebug (tx/mempool-related messages)

  4. TheBlueMatt commented at 4:38 am on March 22, 2012: member
    Everything short of the last commit is part of #771 and is just here because I had that branch checked out when I wrote this, plus I dont expect this to get merged until 0.7 anyway, so there is no point doing it on master. And agreed on the more finely controlled Debug messages, but AddAddress gets really annoying when you are reading debug.log right after startup (which, if you have an error, its likely to occur closer to startup anyway) Maybe one printf per AddAddress…
  5. sipa commented at 4:55 am on March 22, 2012: member
    @TheBlueMatt addrman has significantly less address messages (it adds groups of incoming addrs at once).
  6. TheBlueMatt commented at 5:22 am on March 22, 2012: member
    Ah, well Ill go drop that commit from CBlockStore then.
  7. Diapolo commented at 3:05 pm on March 22, 2012: none
    The strings explaining -maxreceivebuffer and -maxsendbuffer should be updated to reflect the change of the default size.
  8. TheBlueMatt commented at 4:56 pm on March 22, 2012: member
    Oops, good point…fixed.
  9. Diapolo commented at 5:26 pm on March 22, 2012: none
    Helping and get helped, right ;).
  10. TheBlueMatt closed this on Mar 26, 2012

  11. TheBlueMatt reopened this on Mar 27, 2012

  12. gavinandresen commented at 6:30 pm on April 2, 2012: contributor

    The “don’t disconnect” part of this change worries me.

    If an attacker can connect to you 100 times from 100 different IP addresses and then fill up the send buffer on each connection that’s bad.

    I suppose they could try to fill up your send buffer minus 1 byte now…

  13. TheBlueMatt commented at 8:59 pm on April 2, 2012: member
    This patch doesnt change the difficulty of eating 10MB of any node’s ram. Also, I dont see much option in the way of disconnecting nodes for eating too much ram in buffers, we may end up just killing nodes behind 24.4k modems. Or atleast I dont trust myself enough to write any kind of such code.
  14. jgarzik commented at 11:29 pm on May 8, 2012: contributor
    ACK
  15. TheBlueMatt commented at 8:48 pm on June 6, 2012: member
    Rebased.
  16. sipa commented at 7:00 pm on June 12, 2012: member

    What if a node sends you a good new latest best block while the send buffer to that node is full, and then disconnects?

    Probably not a real issue, and I certainly likes this approach more than the earlier hack to cut off the 500 block invs earlier when size exceeds the buffer.

  17. sipa commented at 3:27 pm on June 14, 2012: member
    ACK
  18. Stop processing messages on full send buffer and dont disconnect.
    Also decrease default send/receive buffer sizes from 10 to 5 mb
    as this patch makes it easy for a node to fill both instead of
    only send.
    9d6cd04b3b
  19. gavinandresen commented at 2:42 pm on June 27, 2012: contributor
    ACK
  20. sipa referenced this in commit 6c88568fef on Jun 27, 2012
  21. sipa merged this on Jun 27, 2012
  22. sipa closed this on Jun 27, 2012

  23. coblee referenced this in commit a402cd6909 on Jul 17, 2012
  24. jwrb referenced this in commit 75d3ff7b56 on Sep 17, 2015
  25. jwrb referenced this in commit e220d874e4 on Oct 22, 2015
  26. jwrb referenced this in commit f63ee425fc on Oct 22, 2015
  27. suprnurd referenced this in commit d48fe533d6 on Dec 5, 2017
  28. lateminer referenced this in commit 9de90168f9 on Jan 22, 2019
  29. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 13:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me