Re-introduce new "demand-based" bandwidth limiting - #43
Draft
jacquesh wants to merge 1 commit into
Draft
Conversation
This was previously introduced internally but was stripped out as we ran into a strange issue during deployment and did not have the capacity to investigate it at the time. This commit re-introduces it, although some of the required functionality was already present and simply not used, so as to ease the process of re-introducing it. As a high-level overview, the existing/older form of bandwidth limiting is fully reactive: it does nothing until a given user is detected to have exceeded their transfer limit for a given second, and then prevents them from transferring anything at all for the rest of that second (and some time into the next second to compensate for exceeding the limit). By contrast the newer form of bandwidth limiting is somewhat proactive: we instead enforce an ongoing rate limit locally for each user in each HAProxy instance (using the same logic as the built-in bwlim filter). This locally-enforced limit starts out at some default but is updated regularly by polygen based on the number of concurrent transfers being processed for a particular user across the cluster at a time.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
This was previously introduced internally but was stripped out as we ran into a strange issue during deployment and did not have the capacity to investigate it at the time. This commit re-introduces it, although some of the required functionality was already present and simply not used, so as to ease the process of re-introducing it.
As a high-level overview, the existing/older form of bandwidth limiting is fully reactive: it does nothing until a given user is detected to have exceeded their transfer limit for a given second, and then prevents them from transferring anything at all for the rest of that second (and some time into the next second to compensate for exceeding the limit).
By contrast the newer form of bandwidth limiting is somewhat proactive: we instead enforce an ongoing rate limit locally for each user in each HAProxy instance (using the same logic as the built-in bwlim filter). This locally-enforced limit starts out at some default but is updated regularly by polygen based on the number of concurrent transfers being processed for a particular user across the cluster at a time.
I'm creating this as a draft PR just so that we have it here and it's easy to find when we get around to trying to get this back in again (although I'm sure it will require another rebase at that time). Another reason I put this up now is that I suspect that #42 fixes the issue that we ran into last time we enabled it (but didn't have the time to investigate then). I do not expect this to get any significant review or be merged imminently.
Type of Change
Checklist