From 965f0bc861da0d4478071aeca0a5f2c2202257b7 Mon Sep 17 00:00:00 2001 From: Chashika Weerathunga Date: Fri, 11 Aug 2023 21:27:21 +0530 Subject: [PATCH] Fix incrementing cache keys when overall status is over limited (#430) Signed-off-by: chashikajw Signed-off-by: Tharsanan1 --- src/redis/fixed_cache_impl.go | 13 +++++++++++-- test/integration/integration_test.go | 5 +++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/redis/fixed_cache_impl.go b/src/redis/fixed_cache_impl.go index 2dbb04ff..6f134d53 100644 --- a/src/redis/fixed_cache_impl.go +++ b/src/redis/fixed_cache_impl.go @@ -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 == "" { @@ -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 } @@ -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) } } diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index c8861a3a..c951d1ab 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -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