Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible to implement ratelimiting #2489

Open
slothbag opened this issue Mar 21, 2016 · 14 comments
Open

Possible to implement ratelimiting #2489

slothbag opened this issue Mar 21, 2016 · 14 comments
Labels
kind/enhancement A net-new feature or improvement to an existing feature

Comments

@slothbag
Copy link

I have an urgent need to have a rate limited ipfs node.. perhaps implementing something like this https://github.com/juju/ratelimit within IPFS

Would it be possible to have a user configurable option in ipfs to enact this rate limit logic, if not activated carry on as it currently does?

Can one of the devs provide an estimate on how hard this would be to implement (Time/Cost)? Perhaps I can organise a bounty for it to get it bumped up the priority list :)

Much appreciated.

@slothbag slothbag changed the title Implement ratelimiting Possible to implement ratelimiting Mar 21, 2016
@slothbag
Copy link
Author

Just thinking out loud, to implement rate limiting you'd also have to manage the peer count.. if you restrict the bandwidth to say 10KB/sec you'd probably have to limit your peers to say 10 to give them at least 1KB/sec each.

@whyrusleeping
Copy link
Member

rate limiting wouldnt be too difficult to do, but i'm not sure the effect it would have on overall performance yet. My thoughts are that things will become verrrrry slow if we start rate limiting things..

If you still want to go ahead and try it, take a look at the metrics stuff in libp2p https://github.com/ipfs/go-libp2p/blob/master/p2p/metrics/conn/conn.go

Every connection in ipfs gets wrapped in one of those, and its where we record our (albeit seeminly incorrect) bandwidth stats. You could very easily implement your own bandwidth recorder type object that does token bucket rate limiting, and pass that into the swarm constructor here: https://github.com/ipfs/go-ipfs/blob/master/core/core.go#L140

@slothbag
Copy link
Author

Thanks for the tips. I guess one defence of rate limiting is just about every bittorrent client supports rate limiting, but the network still performs downloads at peak speeds with enough nodes.

You mentioned the bandwidth stats are seemingly wrong, how do you view those stats?

@whyrusleeping
Copy link
Member

you can run ipfs stats bw to see them. The command has a few options for polling and filtering by peer/protocol

@slothbag
Copy link
Author

Thanks, yeah it looks like the accounting is under stating the bandwidth per/sec by at least 50%

@whyrusleeping
Copy link
Member

Yeah, i'm not sure what the discrepancy is... If you want to take a look at it, please do. I'd love a second pair of eyes on it all

@slothbag
Copy link
Author

slothbag commented May 16, 2016

I've been playing around with replicating the wire protocol and found that go-multistream is sending message size prefixes and line feeds "\n" as separate packets.. you can see the code here https://github.com/whyrusleeping/go-multistream/blob/master/multistream.go#L37 and I can confirm that wireshark is showing multiple packets coming in with 1 byte of data.

I wonder if this is part of the cause for incorrect bandwidth usage stats? The overheads for sending a packet with 1 byte instead of say 1400 bytes would cause quite a big difference. Traffic monitoring programs show the total bytes including packet overheads while go-ipfs is probably just reporting the data bytes..

@whyrusleeping whyrusleeping added this to the Resource Constraints milestone Aug 9, 2016
@whyrusleeping
Copy link
Member

@slothbag I just now saw this comment. Thats very interesting... thank you for pointing that out

@jbenet
Copy link
Member

jbenet commented Aug 10, 2016

Wow. That's certainly possible, given how the go net library works and how
we're using it. Our writes go all the way down. We probably want a
flushable stream behavior either manual or time aggregated.

For a lot of this network code, aggregating every 1ms would do well in most
networks (wherever the nic speed is slower than that), if the overhead of
time aggregation is <100us. That might be easier than threading flushes
through all the streams interfaces.

Sadly, an io.RWC isn't everything you ever hoped to do with a network pipe.

I bet Someday, people will implement this as an ML based io scheduler that
optimizes over learned patterns.

On Tue, Aug 9, 2016 at 12:29 Jeromy Johnson notifications@github.com
wrote:

@slothbag https://github.com/slothbag I just now saw this comment.
Thats very interesting... thank you for pointing that out


You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub
#2489 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAIcodCfn759FIcoTIb7xQrRYr8Vcorrks5qeKsAgaJpZM4H0zz-
.

@whyrusleeping
Copy link
Member

I actually prefer go's method of doing things over how C does it. In C you're never really sure when you need to flush out your file descriptors, or when you need to manually buffer things up. Go always pushes writes through by default, there it no buffering on file descriptors (unlike the *FILE in C) and you're required to do it yourself. This means that we have to think about buffering and do it where it makes sense. muuuuch less error prone.

@Kubuxu
Copy link
Member

Kubuxu commented Aug 11, 2016

I've ran a bit of statistical analysis on IPFS packets capture (only data packets, PUSH flag on, no ACK only).
The mean size of packet is 110 bytes, median 44 and most packets (40%) are 4 bytes long.
This results in high overhead of Eth/IP/TCP headers (and ACKs), the overhead is 66 bytes per packet (Eth 14, IPv4 20, TCP 32).

Histograms:

sizes_full
sizes_200

@Kubuxu
Copy link
Member

Kubuxu commented Aug 11, 2016

By default Go uses NoDelay on TCP connections, I think disabling it could result in major savings on number of packets (and thus overhead) we send.

@whyrusleeping is that a good idea? NoDelay is buried deep (not exposed from go-reuseport, then we would have to pass option through multiaddr) and I have no idea how to resurface it.

@whyrusleeping
Copy link
Member

I really don't think NoDelay is a good solution. It will result it weird random hangs and slowdowns for us.

For instance, the final packet of a write the spans multiple packets will be held back until an ACK is received from the other side. This will incur hundreds of milliseconds of latency in simple RPCs. This will be especially painful in things like dht requests, where the responses can easily be larger than a single packet.

@em-ly em-ly added the kind/enhancement A net-new feature or improvement to an existing feature label Aug 25, 2016
@Kubuxu
Copy link
Member

Kubuxu commented Aug 30, 2016

@whyrusleeping made awesome changes to our netstack libriaries that should reduce number of packets sent, I will repeat my tests.

Also I will open separate issue for just this metric as packet size it is something we should aim to max out in a long run to increase bandwidth usage efficiency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants