Allow capping of bandwidth on overall or command-specific basis, reset via cron-like expression #2479

pull brandondahler wants to merge 1 commits into bitcoin:master from brandondahler:bandwidth-cap changing 12 files +1269 −75
  1. brandondahler commented at 6:24 AM on April 7, 2013: contributor

    This is a first-pass change to begin resolving issues and #273, #1958, #2412. I doubt this change completely ready, I intend on using this request for feedback on the current implementation of code and usage.


    Basic

    Allow users to set bandwidth limits on overall egress and/or command-specific egress bandwdith. Overall egress bandwidth and command-specific egress bandwidth counts are saved to disk (currently every packet queued), to allow counts to be saved across runnings of the bitcoin node. A simplified cron-expression is used to determine when to reset all bandwidth counts.

    The cron-expression will cause start and reset times to "snap" to the closest values above and below the current time. This allows exact control over when the bandwidth usage is reset via the expression, instead of relying on the time the program was last started or other arbitrary limitations.


    Configuration options

    For command-line (prepend '-') or configuration file:

    • bandwidthsentmax - Overall egress bandwidth limit
    • bandwidthsentmax_[command] - Command-specific bandwidth limit
    • bandwidthresettimes - Cron-expression used to reset bandwidth counts

    Formats

    • Bandwidth - As regular expression /[0-9]+[BbKkMmGgTtPp]?[Bb]?/ - Valid for any values 0 - 18,446,744,073,709,551,616 bytes (16,384 Peta-bytes)
    • Cron - See wiki for full cron expressions. Numerical representations allowed only. Special characters implemented: asterisk (*), slash (/) (with caveats), comma (,), and hyphen (-). Slash requires the end value be landed upon (which means */4 requires the possible range be divisible by 4).

    Sample configuration

    Bitcoin.conf

    # minutes      hours       dayOfMonth  Month   dayOfWeek
    # Resets each month
    [#0](/bitcoin-bitcoin/0/)            0           1           *       *
    #
    # Reset every sunday
    # *           *           *           *       0
    # Resets each minute
    # *           *           *           *       *
    #
    # Resets every 5 minutes
    # */5          *           *           *       *           
    bandwidthresettimes=0 0 0 * *
    
    # Only allow 2 GByte of sent data
    bandwidthsentmax=2G
    
    # Only send out 1 GByte of sent "block" commands
    bandwidthsentmax_block=1G
    

    Files this change-set:

    New

    bandwidthman.h and bandwdithman.cpp

    • New class to handle bandwidth management as needed, will serve a larger purpose as bandwidth management features are expanded.
    • Public functions
      • LoadConfiguration() - Uses mapArgs to read reset time cron expression, uses mapArgs to read overall bandwidth cap and specific command bandwidth caps.
      • TestScheduling(string strCronLine, struct tm tmTestTime) - Parses a given cron expression and uses a specific time to calculate start time and reset time. Used only by the unit tester.
      • AllowSendCommand(string strCommand, int nSize) - Determines if strCommand should be sent, given the command is nSize.

    tests/bandwidthman_tests.cpp

    • Contains unit tests for the calculation of start times and reset times used by the CBandwidthMan class.

    Changed

    db.h

    • Rename CAddrDB to CDatDB and make abstract, generalize specific functions to globally applicable functions, consider the general format used by CAddrDB as being a ".dat" file.
    • Create templated wrapper for new CDatDB named CSerialDatDB. Extends CDatDB and allows for any serializable type to be used as the template type.
    • Create CAddrDB by making it an alias of CSerialDatDB<CAddrMan>, construct the CSerialDatDB with the static filename of "peers.dat" (as previously hard-coded).
    • Create CBandwidthDB by making it an alias of CSerialDatDB<CBandwidthMan>, construct CSerialDatDB with the static filename of "bandwidth.dat".

    Note: All changes to CAddrDB result in no usability changes.

    db.cpp

    • Change CAddrDB functions to CDatDB functions.
    • Modify write process to prepend message start and append hash due to change in serialization time of payload data (new lines 488-494).

    init.cpp

    • Insert new step 11: Load bandwidth manager.
    • Renumber old steps 11 and 12 to 12 and 13 respectively.

    makefile.*

    • Add objs/bandwidthman.o as new object

    net.h

    • Export instance of bandwidth manager.
    • Modify EndMessage to require the message name upon invocation and return success or failure to send message.
      • EndMessage will return false if it is not allowed to send the message as determined by the bandwidth manager.
      • If bandwidth manager does not allow the message to be sent, abort message.
    • Modify all push messages to return success or failure, determined by EndMessage call.
      • Success/failure result is not currently used but planned for use later.

    net.cpp

    • Add new global instance of CBandwidthMan.

    Extra data

    While researching how to approach the problem, I collected command-specific egress data off of an unrestricted node that was on the latest block. This spreadsheet shows the count and total bandwidth of each of the commands. I do not remember when or how long it took to collect this data, but I believe it was over a few hours with no local usage of the computer. The branch at brandondahler/bitcoin branch bandwidth-test can produce a text file that can aid in plugging in your own data into this spreadsheet.

  2. Add bandwidth manager to allow capping of outbound bandwidth, allow resetting of statistics based on a cron-like expression. 1966c325a4
  3. BitcoinPullTester commented at 6:39 AM on April 7, 2013: none

    Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/1966c325a4e5e9de7c89f98869663272d33fab94 for binaries and test log.

    This could happen for one of several reasons:

    1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (you can find the patches applied at test-time at http://jenkins.bluematt.me/pull-tester/files/patches/)
    2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
    3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
    4. The test suite fails on either Linux i386 or Win32
    5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

    If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

    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.

  4. brandondahler commented at 6:52 AM on April 7, 2013: contributor

    Build failure above appears to be a false-positive--the patch "bitcoind-comparison.patch" needs to be updated to be able to apply to commits after 8c222dca4f961ad13ec64d690134a40d09b20813.

  5. yhaenggi commented at 7:22 AM on April 7, 2013: none

    this is just fking awesome! thanks :)

  6. gmaxwell commented at 7:30 AM on April 7, 2013: contributor

    Very cool that you're working on this. But unless I'm reading this wrong, it looks like when it hits the limit it will just stop sending messages to peers. This would break the network and its not something we can merge.

    As you note in your spreadsheet, virtually all the bandwidth usage is relaying blocks— and I expect that if you instrumented for it you'd find it was from relaying historic ones. This is why I've been recommending to people that they make their nodes not listen to inbound connections when they want to limit their usage. (nodes don't accept inbound connections until they have most of the chain, so a node you connect out to will almost never pull any of the history from you)

    What I'd suggest here is that you instead close inbound peers and refuse non-localhost p2p connections once the limit is hit, and don't do addr broadcasts when the limit is anywhere close.

    This won't help for the minute/hour timescales, however— but we can't take limiting on those timescales until the initial block download process is changed so that having limits on those scales won't break node initialization. (though that may happen soon)

    Make sense?

  7. mikehearn commented at 3:35 PM on April 7, 2013: contributor

    Yeah, you can't just not respond to getblocks requests, that will break things (today). Even just serving it very slowly won't work well (the peers won't switch to a faster node).

    Long term we need some better mechanisms for network load balancing. If a node is running out of resources, it'd be good if it could redirect peers to other nodes that have more capacity. For that we need an upgrade to the protocol though, so nodes can advertise their available capacity. It's a harder change. Just closing connections like gmaxwell suggests is a good start. Of course, if everyone uses this feature, then we run the risk of the network running out of upload bandwidth!

  8. mikehearn commented at 3:37 PM on April 7, 2013: contributor

    You could also stop setting the version services bit indicating you can serve the chain. That way peers will think you're an SPV node and won't ask for the chain.

  9. brandondahler commented at 6:11 PM on April 7, 2013: contributor

    @gmaxwell @mikehearn : Is there currently a proper way to refuse a getblocks request? Right now there is a mechanism that will allow callers of the PushMessage functions to see if they failed to be sent or not, having them send a different response based on that is trivial.

    It sounds like closing all connections will be a good idea for nodes that have met their overall max bandwidth. I will look in to getting that added in the next weeks.

  10. sipa commented at 6:50 PM on April 7, 2013: member

    I think the only current way to not respond to getblocks (and getdata, which is the actual expensive part) is not advertizing that you do in the first place. Which indeed means disconnecting or not setting NODE_NETWORK, right now.

    This discussion will probably be needed soon anyway, if we want to move forward with pruned nodes (which only store part of the block chain, and may only relay parts or nothing at all).

  11. jgarzik commented at 5:19 AM on April 8, 2013: contributor

    Bandwidth limits are a common P2P knob, well within reasonable user expectations. It has been said for years, in the Linux kernel, that it was awful to put bandwidth limits inside each application, when instead smarter, higher level layers such as cgroups and tc/qdisc would be far more appropriate.

    In theory, that is true. In practice, it is easier to deploy immediately in a user application. In-app limits do not require modifying kernel or firewall configuration -- which the user may not have rights to access anyway.

    Therefore, I respectfully disagree with @gmaxwell. Closing inbound peers is a very poor solution, and it is disappointing to see that as a recommendation -- when the alternative is to enable users with a limit commonly found in other P2P applications.

    RE @mikehearn: "Even just serving it very slowly won't work well (the peers won't switch to a faster node)." @sipa is already starting to address this, by scoring peers in #2461. As such, serving slowly is preferred to serving nothing. That enables 50% of the solution -- with the other 50% of the solution being better seeking/scoring of peers for getblocks requests. That sets up a natural ecosystem: peers do what they can, and other peers seek the best behaving peers. At times when our popular peers are over-loaded, the less able peers automatically step in and provide some level of service.

  12. jgarzik commented at 5:22 AM on April 8, 2013: contributor

    Also, bandwidth limits simply makes the local node's behavior more predictable. You can guarantee that your VPS does not exceed its bandwidth budget. You may guarantee that bitcoind will never burst sufficiently to disrupt your game of Star Wars: The Old Republic. There is nothing wrong with users that want this accounting ability.

    See Tor's torrc for one very flexible method of specifying bandwidth contributions.

  13. jgarzik commented at 5:28 AM on April 8, 2013: contributor

    To be clear, NAK on anything that drops getblocks requests, or similar. As @mikehearn said, "you can't just not respond to getblocks requests, that will break things"

  14. gmaxwell commented at 5:57 AM on April 8, 2013: contributor

    I am absolutely not opposed to having bandwidth limits (in whatever structure people want them in— Tor has both short timescale and long term accounting limits). I have no idea how Jeff got the idea that I (or anyone else here) doesn't fully support having them.

    We just cannot have them yet, as I said: "but we can't take limiting on those timescales until the initial block download process is changed so that having limits on those scales won't break node initialization. (though that may happen soon)".

    For the monthly accounting limits switching out of listening/node-network state is a possible viable option. Though it only works for the long timescale limits (Similar to the AccountingMax setting in Tor), not the "keep traffic down to avoid messing up my games". I don't see why Jeff would consider that disappointing, it's certainly not something complete— but its also something that could be deployed in today's network. Any other limits will need to be at least two versions out (the version after IBD loadbalancing is fixed).

  15. jgarzik commented at 6:09 AM on April 8, 2013: contributor

    Even a simple kb/sec upload limit would be fine. There is no need for that to be N versions out.

    Bandwidth limiting occurs naturally on the Internet. Therefore, it seems exaggerated to suggest that an optional limit, mimicing a found-in-nature condition, will break much of anything.

  16. gmaxwell commented at 6:11 AM on April 8, 2013: contributor

    @jgarzik Right now "bitcoin took two (or more) days to synchronize" is the most common complaint I hear about the software. This is a direct result of fetching from low bandwidth peers. I do not think that we can afford to make that worse before we make it better.

  17. jgarzik commented at 6:22 AM on April 8, 2013: contributor

    "make that worse" is a severely constrained point of view. Blocking a user from better control over their own private property (by arguing against bandwidth limits commits) is the wrong way to achieve that goal. The net result discourages people from running peers at all, because the default software is less predictable, more susceptible to the whims of remote peers.

    "make that worse" implies selection bias and ignores opportunity cost -- excluding the bandwidth contributed by discouraged peers, and focusing mainly on the existing set of peers on the network as if a static entity.

  18. gmaxwell commented at 6:28 AM on April 8, 2013: contributor

    @jgarzik I fully support people controlling their own systems. [Edit: I've toned this down a bit. I know Jeff doesn't intend to insult me, and if I've some how left him thinking that I want to restrict choice then I've been doing a very poor job expressing myself, and thats my fault not his]

    Right now users can stop the high bandwidth usage by simply disabling listening. At the moment, because of the way nodes select and use peers, this is preferable to having listening enabled but artificially rate limited—" don't advertise a capacity you don't have because your peers aren't smart enough to figure out that you don't and cope with it". It's not ideal but it will prevent the IBD badness. A limiter that also turned off node-network when set under some value would likewise by completely fine.

    Since you used Tor as an example— it would be useful to consider all the work the Tor project has put into load-balancing: Tor nodes report their configured/observed bandwidth to Tor directory authorities, the bandwidth authorities (https://gitweb.torproject.org/torflow.git/blob/HEAD:/NetworkScanners/BwAuthority/README.spec.txt) periodically test nodes to make sure their bandwidth claims are honest, and then only the top 7/8th's of nodes get the flagged ("fast") for primary use. There used to be a minimum bandwidth threshold under which nodes were ignored because they degraded the network, but that become superfluous to the 7/8ths test. Load balancing is then linear on bandwidth. But linear load-balancing doesn't actually achieve the best utilization, so they've experimented with things like using a PID controller (https://lists.torproject.org/pipermail/tor-relays/2011-December/001039.html) to weigh the bandwidth amounts to achieve better utilization, but it seems they've given up on this particular mechanism— I mention it as color for how complicated a problem this is, especially when you need to be attack resistant.

    Fortunately, our application is not quite as sybil vulnerable as Tor... but unfortunately we don't get to have central authorities telling people validated bandwidths. It's also interesting to note that Tor has seen some parallel arguments to the ones made against my recommendation to turn off listening— that its demotivating to people who want to contribute— see the last comment (By cyberpunks/paul) at https://trac.torproject.org/projects/tor/ticket/1854

  19. sipa commented at 8:46 AM on April 8, 2013: member

    @jgarzik @gmaxwell I think you're talking about different things.

    Nobody is saying that bandwidth control in general is a bad idea. It's a common expectation from P2P software, and a very reasonable one.

    However, with the current software, effectively the first peer connected to is being used to fetch blocks from. I consider that a bug, and it needs fixing independently from this issue. Nonetheless, it means that even when (from what I can tell) more than enough fast upload capacity is available from the network, users face horribly slow block download because they randomly connect to a peer who probably doesn't even like the fact that he's serving the historic chain in the first place.

    Right now, with the state of the current software, and without changing the protocol, I do agree that simply dropping the connection is the best throttling mechanism we have, as that will result in the peer (eventually, but see #2461) finding a new peer instead of waiting days before every block is uploaded through - who knows - perhaps a GPRS connection. Dropping the connection results in a better experience for both parties.

  20. mikehearn commented at 8:47 AM on April 8, 2013: contributor

    Yeah, I'm all for a smarter networking protocol too - it's just that this patch doesn't do that, as is.

    Also, bear in mind that if the networking protocol changes, everything that talks it has to be upgraded. Currently bitcoinj scores peers by ping time (amongst other things), because peers that are very slow/busy serving the chain to someone else/overloaded/bandwidth-saturated tend to have high ping times. If a peer has low ping times but still responds to getblocks slowly, then that's fine but I'll have to update that code and users need to be updated.

  21. gavinandresen commented at 5:56 PM on April 9, 2013: contributor

    No consensus. Closing.

  22. gavinandresen closed this on Apr 9, 2013

  23. 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: 2026-04-13 21:16 UTC

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