Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

MegaFix global behavior bugs. #225

Merged
merged 24 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f02cb5b
Fix global behavior `ResetTime` bug.
Baliedge Feb 26, 2024
a665c3c
Refine request time propagation.
Baliedge Feb 28, 2024
427dd2b
Merge branch 'master' of github.com:mailgun/gubernator into Baliedge/…
Baliedge Feb 28, 2024
57a5c97
Fix race condition in global behavior.
Baliedge Mar 6, 2024
56a0b22
Fix compile error.
Baliedge Mar 7, 2024
cb3816a
Fix intermittent test error caused by `TestHealthCheck`.
Baliedge Mar 7, 2024
e2b8853
Refactor global behavior and functional tests for stability.
Baliedge Mar 11, 2024
7c67a32
Fix lint errors.
Baliedge Mar 11, 2024
24bee89
Tidy code.
Baliedge Mar 11, 2024
ea42442
Add back tests that were erroneously removed.
Baliedge Mar 11, 2024
bd38ee6
Fix tests.
Baliedge Mar 11, 2024
65ee4fa
Fix benchmark test errors.
Baliedge Mar 11, 2024
d5c74d2
Fix TestHealthCheck.
Baliedge Mar 11, 2024
cd7bbab
Fix flaky test `TestHealthCheck`.
Baliedge Mar 11, 2024
0fb2a33
Backwards compatibility needed for upgrading.
Baliedge Mar 11, 2024
47eede5
Fix compile error.
Baliedge Mar 11, 2024
2229596
Fix test.
Baliedge Mar 11, 2024
c0608d5
Fix for overlimit metric doublecounting on non-owner and owner.
Baliedge Mar 11, 2024
517bf63
Metric `gubernator_getratelimit_counter` adds calltype value "local n…
Baliedge Mar 11, 2024
5f137ad
Changed mind. Instead of `calltype="local non-owner"`, just don't in…
Baliedge Mar 11, 2024
f51861d
Don't call `OnChange()` event from non-owner.
Baliedge Mar 12, 2024
d55016d
Simplify cache item expiration check.
Baliedge Mar 13, 2024
5ce3bc1
Rename `RequestTime` to `CreatedAt` in protos.
Baliedge Mar 13, 2024
0b2adf6
Revert optimization that won't work.
Baliedge Mar 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Simplify cache item expiration check.
  • Loading branch information
Baliedge committed Mar 13, 2024
commit d55016d2447eaac133c0a5f381fd988abcd5a97d
12 changes: 6 additions & 6 deletions algorithms.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,18 @@ func tokenBucket(ctx context.Context, s Store, c Cache, r *RateLimitReq, reqStat
rl.ResetTime = expire
}

if s != nil && reqState.IsOwner {
defer func() {
s.OnChange(ctx, r, item)
}()
}

// Client is only interested in retrieving the current status or
// updating the rate limit config.
if r.Hits == 0 {
return rl, nil
}

if s != nil && reqState.IsOwner {
defer func() {
s.OnChange(ctx, r, item)
}()
}

// If we are already at the limit.
if rl.Remaining == 0 && r.Hits > 0 {
trace.SpanFromContext(ctx).AddEvent("Already over the limit")
Expand Down
16 changes: 16 additions & 0 deletions cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,19 @@ type CacheItem struct {
// for the latest rate limit data.
InvalidAt int64
}

func (item *CacheItem) IsExpired() bool {
now := MillisecondNow()

// If the entry is invalidated
if item.InvalidAt != 0 && item.InvalidAt < now {
return true
}

// If the entry has expired, remove it from the cache
if item.ExpireAt < now {
return true
}

return false
}
4 changes: 2 additions & 2 deletions gubernator.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func (s *V1Instance) asyncRequest(ctx context.Context, req *AsyncReq) {
funcTimer := prometheus.NewTimer(metricFuncTimeDuration.WithLabelValues("V1Instance.asyncRequest"))
defer funcTimer.ObserveDuration()

reqState := RateLimitReqState{IsOwner: false}
reqState := RateLimitReqState{IsOwner: req.Peer.Info().IsOwner}
resp := AsyncResp{
Idx: req.Idx,
}
Expand All @@ -337,7 +337,7 @@ func (s *V1Instance) asyncRequest(ctx context.Context, req *AsyncReq) {

// If we are attempting again, the owner of this rate limit might have changed to us!
if attempts != 0 {
if req.Peer.Info().IsOwner {
if reqState.IsOwner {
resp.Resp, err = s.getLocalRateLimit(ctx, req.Req, reqState)
if err != nil {
s.log.WithContext(ctx).
Expand Down
11 changes: 1 addition & 10 deletions lrucache.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,7 @@ func (c *LRUCache) GetItem(key string) (item *CacheItem, ok bool) {
if ele, hit := c.cache[key]; hit {
entry := ele.Value.(*CacheItem)

now := MillisecondNow()
// If the entry is invalidated
if entry.InvalidAt != 0 && entry.InvalidAt < now {
c.removeElement(ele)
metricCacheAccess.WithLabelValues("miss").Add(1)
return
}

// If the entry has expired, remove it from the cache
if entry.ExpireAt < now {
if entry.IsExpired() {
c.removeElement(ele)
metricCacheAccess.WithLabelValues("miss").Add(1)
return
Expand Down
Loading