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

Ability to limit/throttle bandwidth #34

Open
MrBruz opened this issue May 20, 2022 · 8 comments
Open

Ability to limit/throttle bandwidth #34

MrBruz opened this issue May 20, 2022 · 8 comments
Labels
enhancement New feature or request
Milestone

Comments

@MrBruz
Copy link

MrBruz commented May 20, 2022

An option to limit bandwidth usage per second or per day/month would be a welcome feature.

@yunginnanet yunginnanet added the enhancement New feature or request label May 21, 2022
@yunginnanet yunginnanet added this to the v0.4 milestone Jun 8, 2022
@yunginnanet
Copy link
Owner

yunginnanet commented Sep 20, 2022

I have begun working on this in the development branch here: https://github.com/yunginnanet/HellPot/blob/development/internal/util/speedometer.go

@yunginnanet
Copy link
Owner

@yunginnanet
Copy link
Owner

btw see here: https://github.com/yunginnanet/Rate5/blob/6fda3907504eb711cd0ab1c8c0c9f398d9008b42/speedometer/speedometer.go

and here: yunginnanet/Rate5#11

its pretty much ready too as far as i can tell but i never have anyone to review my code so i can be a bit hesitant at times

crosspost from #35

@ShadowJonathan
Copy link

just finished development/internal/util/speedometer.go?rgh-link-date=2022-09-20T16%3A09%3A50Z

only need to implement now

this doesn't work properly when the buffer is larger than the frame, it sends the entire first buffer, and then takes a lot of time throttling out the rest, basically stalling

@yunginnanet
Copy link
Owner

just finished development/internal/util/speedometer.go?rgh-link-date=2022-09-20T16%3A09%3A50Z
only need to implement now

this doesn't work properly when the buffer is larger than the frame, it sends the entire first buffer, and then takes a lot of time throttling out the rest, basically stalling

makes sense. the implementation was going to come along with a unit test to assure this works properly, part of why i am not rushing it

likely i may just need to create a non-blocking implementation option within the ratelimiter package and respond to an error within HellPot upon throttle instead of blocking/sleeping on write calls

@yunginnanet
Copy link
Owner

yunginnanet commented Feb 3, 2024

just finished development/internal/util/speedometer.go?rgh-link-date=2022-09-20T16%3A09%3A50Z
only need to implement now

this doesn't work properly when the buffer is larger than the frame, it sends the entire first buffer, and then takes a lot of time throttling out the rest, basically stalling

while this made sense when I initially read it, it'd be helpful to see an example in the form of a go playground share or a test case to properly demonstrate this flaw

tldr; PoC pls

current code is here: https://github.com/yunginnanet/Rate5/tree/main/speedometer

note that here i use a buffer twice as large as the burst value during the TCP testcase:

https://github.com/yunginnanet/Rate5/blob/4a3e80ab1ad46514f3f49f955ac6d061932bc940/speedometer/speedometer_test.go#L285-L389

@ShadowJonathan
Copy link

ShadowJonathan commented Feb 3, 2024

when i set the frame to 1024 bytes and the interval to 1 second, it writes the entire buffer at once, a few times over, before stopping when it meets a "check every byte" boundary

when i made that check work every write, it seems to throttle and stall after the first buffer write

	sp, nerr := util.NewLimitedSpeedometer(bw, &util.SpeedLimit{
		Burst:           1024 * 4,
		Frame:           1 * time.Second,
		CheckEveryBytes: -1,
		Delay:           100 * time.Millisecond,
	})

	if nerr != nil {
		return n, nerr
	}

	if n, err = io.CopyBuffer(sp, h.mm, buf); err != nil {
		return n, nil
	}

(Here i've changed the backend code of the speedlimiter to behave better (by writing buffers in chunks, and checking speed every time), but the current version will not work with this. You might wanna set CheckEveryBytes to 1 or 10 or something, but it'll often skip a few buffer-sized chunks before it sees it, and stall hard.)

@yunginnanet
Copy link
Owner

thank you, i'll look further into this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants