-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat: new stream count limiter #13006
Merged
vlad-diachenko
merged 5 commits into
main
from
vlad.diachenko/owned-stream-count-limiter
May 23, 2024
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
c9b9131
implemented a new stream count limiter that uses the values from owne…
vlad-diachenko 7e984f1
updated the docs
vlad-diachenko 9e9e1b8
fixed imports
vlad-diachenko 8dd26fb
Update pkg/validation/limits.go
vlad-diachenko a850384
updated the docs
vlad-diachenko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ type RingCount interface { | |
|
||
type Limits interface { | ||
UnorderedWrites(userID string) bool | ||
UseOwnedStreamCount(userID string) bool | ||
MaxLocalStreamsPerUser(userID string) int | ||
MaxGlobalStreamsPerUser(userID string) int | ||
PerStreamRateLimit(userID string) validation.RateLimit | ||
|
@@ -76,46 +77,39 @@ func (l *Limiter) UnorderedWrites(userID string) bool { | |
return l.limits.UnorderedWrites(userID) | ||
} | ||
|
||
// AssertMaxStreamsPerUser ensures limit has not been reached compared to the current | ||
// number of streams in input and returns an error if so. | ||
func (l *Limiter) AssertMaxStreamsPerUser(userID string, streams int) error { | ||
// Until the limiter actually starts, all accesses are successful. | ||
// This is used to disable limits while recovering from the WAL. | ||
l.mtx.RLock() | ||
defer l.mtx.RUnlock() | ||
if l.disabled { | ||
return nil | ||
} | ||
|
||
func (l *Limiter) GetStreamCountLimit(tenantID string) (calculatedLimit, localLimit, globalLimit, adjustedGlobalLimit int) { | ||
// Start by setting the local limit either from override or default | ||
localLimit := l.limits.MaxLocalStreamsPerUser(userID) | ||
localLimit = l.limits.MaxLocalStreamsPerUser(tenantID) | ||
|
||
// We can assume that streams are evenly distributed across ingesters | ||
// so we do convert the global limit into a local limit | ||
globalLimit := l.limits.MaxGlobalStreamsPerUser(userID) | ||
adjustedGlobalLimit := l.convertGlobalToLocalLimit(globalLimit) | ||
globalLimit = l.limits.MaxGlobalStreamsPerUser(tenantID) | ||
adjustedGlobalLimit = l.convertGlobalToLocalLimit(globalLimit) | ||
|
||
// Set the calculated limit to the lesser of the local limit or the new calculated global limit | ||
calculatedLimit := l.minNonZero(localLimit, adjustedGlobalLimit) | ||
calculatedLimit = l.minNonZero(localLimit, adjustedGlobalLimit) | ||
|
||
// If both the local and global limits are disabled, we just | ||
// use the largest int value | ||
if calculatedLimit == 0 { | ||
calculatedLimit = math.MaxInt32 | ||
} | ||
return | ||
} | ||
|
||
if streams < calculatedLimit { | ||
return nil | ||
func (l *Limiter) minNonZero(first, second int) int { | ||
if first == 0 || (second != 0 && first > second) { | ||
return second | ||
} | ||
|
||
return fmt.Errorf(errMaxStreamsPerUserLimitExceeded, userID, streams, calculatedLimit, localLimit, globalLimit, adjustedGlobalLimit) | ||
return first | ||
} | ||
|
||
func (l *Limiter) convertGlobalToLocalLimit(globalLimit int) int { | ||
if globalLimit == 0 { | ||
return 0 | ||
} | ||
|
||
// todo: change to healthyInstancesInZoneCount() once | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
// Given we don't need a super accurate count (ie. when the ingesters | ||
// topology changes) and we prefer to always be in favor of the tenant, | ||
// we can use a per-ingester limit equal to: | ||
|
@@ -131,12 +125,53 @@ func (l *Limiter) convertGlobalToLocalLimit(globalLimit int) int { | |
return 0 | ||
} | ||
|
||
func (l *Limiter) minNonZero(first, second int) int { | ||
if first == 0 || (second != 0 && first > second) { | ||
return second | ||
type supplier[T any] func() T | ||
|
||
type streamCountLimiter struct { | ||
tenantID string | ||
limiter *Limiter | ||
defaultStreamCountSupplier supplier[int] | ||
ownedStreamSvc *ownedStreamService | ||
} | ||
|
||
var noopFixedLimitSupplier = func() int { | ||
return 0 | ||
} | ||
|
||
func newStreamCountLimiter(tenantID string, defaultStreamCountSupplier supplier[int], limiter *Limiter, service *ownedStreamService) *streamCountLimiter { | ||
return &streamCountLimiter{ | ||
tenantID: tenantID, | ||
limiter: limiter, | ||
defaultStreamCountSupplier: defaultStreamCountSupplier, | ||
ownedStreamSvc: service, | ||
} | ||
} | ||
|
||
return first | ||
func (l *streamCountLimiter) AssertNewStreamAllowed(tenantID string) error { | ||
streamCountSupplier, fixedLimitSupplier := l.getSuppliers(tenantID) | ||
calculatedLimit, localLimit, globalLimit, adjustedGlobalLimit := l.getCurrentLimit(tenantID, fixedLimitSupplier) | ||
actualStreamsCount := streamCountSupplier() | ||
if actualStreamsCount < calculatedLimit { | ||
return nil | ||
} | ||
|
||
return fmt.Errorf(errMaxStreamsPerUserLimitExceeded, tenantID, actualStreamsCount, calculatedLimit, localLimit, globalLimit, adjustedGlobalLimit) | ||
} | ||
|
||
func (l *streamCountLimiter) getCurrentLimit(tenantID string, fixedLimitSupplier supplier[int]) (calculatedLimit, localLimit, globalLimit, adjustedGlobalLimit int) { | ||
calculatedLimit, localLimit, globalLimit, adjustedGlobalLimit = l.limiter.GetStreamCountLimit(tenantID) | ||
fixedLimit := fixedLimitSupplier() | ||
if fixedLimit > calculatedLimit { | ||
calculatedLimit = fixedLimit | ||
} | ||
return | ||
} | ||
|
||
func (l *streamCountLimiter) getSuppliers(tenant string) (streamCountSupplier, fixedLimitSupplier supplier[int]) { | ||
if l.limiter.limits.UseOwnedStreamCount(tenant) { | ||
return l.ownedStreamSvc.getOwnedStreamCount, l.ownedStreamSvc.getFixedLimit | ||
} | ||
return l.defaultStreamCountSupplier, noopFixedLimitSupplier | ||
} | ||
|
||
type RateLimiterStrategy interface { | ||
|
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package ingester | ||
|
||
import "go.uber.org/atomic" | ||
|
||
type ownedStreamService struct { | ||
tenantID string | ||
limiter *Limiter | ||
fixedLimit *atomic.Int32 | ||
|
||
//todo: implement job to recalculate it | ||
ownedStreamCount *atomic.Int64 | ||
} | ||
|
||
func newOwnedStreamService(tenantID string, limiter *Limiter) *ownedStreamService { | ||
svc := &ownedStreamService{ | ||
tenantID: tenantID, | ||
limiter: limiter, | ||
ownedStreamCount: atomic.NewInt64(0), | ||
fixedLimit: atomic.NewInt32(0), | ||
} | ||
svc.updateFixedLimit() | ||
return svc | ||
} | ||
|
||
func (s *ownedStreamService) getOwnedStreamCount() int { | ||
return int(s.ownedStreamCount.Load()) | ||
} | ||
|
||
func (s *ownedStreamService) updateFixedLimit() { | ||
limit, _, _, _ := s.limiter.GetStreamCountLimit(s.tenantID) | ||
s.fixedLimit.Store(int32(limit)) | ||
} | ||
|
||
func (s *ownedStreamService) getFixedLimit() int { | ||
return int(s.fixedLimit.Load()) | ||
} | ||
|
||
func (s *ownedStreamService) incOwnedStreamCount() { | ||
s.ownedStreamCount.Inc() | ||
} | ||
|
||
func (s *ownedStreamService) decOwnedStreamCount() { | ||
s.ownedStreamCount.Dec() | ||
} |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this go in the
LogStreamCreation
conditional?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, we need to do it all the time... btw, even if UseOwnedStreamCount is set to false, because this option is in runtime config and can be turned on at any moment, the service should always know the count of the streams.