Skip to content

Commit

Permalink
Fix incrementing cache keys when overall status is over limited (#430)
Browse files Browse the repository at this point in the history
Signed-off-by: chashikajw <[email protected]>
Signed-off-by: Tharsanan1 <[email protected]>
  • Loading branch information
chashikajw authored Aug 11, 2023
1 parent e059638 commit 965f0bc
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/redis/fixed_cache_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ func (this *fixedRateLimitCacheImpl) DoLimit(
results := make([]uint32, len(request.Descriptors))
var pipeline, perSecondPipeline Pipeline

hitAddendForRedis := hitsAddend
overlimitIndex := -1
// Now, actually setup the pipeline, skipping empty cache keys.
for i, cacheKey := range cacheKeys {
if cacheKey.Key == "" {
Expand All @@ -67,6 +69,13 @@ func (this *fixedRateLimitCacheImpl) DoLimit(
logger.Debugf("cache key is over the limit: %s", cacheKey.Key)
}
isOverLimitWithLocalCache[i] = true
hitAddendForRedis = 0
overlimitIndex = i
continue
}
}
for i, cacheKey := range cacheKeys {
if cacheKey.Key == "" || overlimitIndex == i {
continue
}

Expand All @@ -82,12 +91,12 @@ func (this *fixedRateLimitCacheImpl) DoLimit(
if perSecondPipeline == nil {
perSecondPipeline = Pipeline{}
}
pipelineAppend(this.perSecondClient, &perSecondPipeline, cacheKey.Key, hitsAddend, &results[i], expirationSeconds)
pipelineAppend(this.perSecondClient, &perSecondPipeline, cacheKey.Key, hitAddendForRedis, &results[i], expirationSeconds)
} else {
if pipeline == nil {
pipeline = Pipeline{}
}
pipelineAppend(this.client, &pipeline, cacheKey.Key, hitsAddend, &results[i], expirationSeconds)
pipelineAppend(this.client, &pipeline, cacheKey.Key, hitAddendForRedis, &results[i], expirationSeconds)
}
}

Expand Down
5 changes: 5 additions & 0 deletions test/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,11 @@ func testBasicBaseConfig(s settings.Settings) func(*testing.T) {
if i >= 10 {
status = pb.RateLimitResponse_OVER_LIMIT
limitRemaining2 = 0
// Ceased incrementing cached keys upon exceeding the overall rate limit in the Redis cache flow.
// Consequently, the remaining limit should remain unaltered.
if enable_local_cache && s.BackendType != "memcache" {
limitRemaining1 = 9
}
}
durRemaining1 := response.GetStatuses()[0].DurationUntilReset
durRemaining2 := response.GetStatuses()[1].DurationUntilReset
Expand Down

0 comments on commit 965f0bc

Please sign in to comment.