Apply rate limiting before singleflight to prevent goroutine blocking on etcd timeout (#2841)

* Initial plan

* Apply rate limiting before singleflight to prevent blocking

- Check rate limiting BEFORE entering singleflight
- If rate-limited AND stale cache exists, return stale cache immediately
- This prevents all goroutines from blocking when etcd is down/slow
- Maintains stampede prevention via singleflight for non-rate-limited requests
- All existing tests pass

Co-authored-by: asim <17530+asim@users.noreply.github.com>

* Fix variable shadowing in rate limiting check

- Rename shadowed variables to currentLastRefresh and currentMinimumRetryInterval
- Improves code clarity and prevents potential bugs
- All tests still pass

Co-authored-by: asim <17530+asim@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: asim <17530+asim@users.noreply.github.com>
This commit is contained in:
Copilot
2026-02-03 11:35:34 +00:00
committed by GitHub
co-authored by asim copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
parent 87cf988e03
commit adc90b4d2d
+22 -4
View File
@@ -159,14 +159,22 @@ func (c *cache) get(service string) ([]*registry.Service, error) {
return cp, nil
}
// Check rate limiting before unlocking
// This prevents multiple sequential attempts within the retry interval
// Check rate limiting BEFORE entering singleflight
// This prevents blocking when we have stale cache and etcd is down
lastRefresh := c.lastRefreshAttempt[service]
minimumRetryInterval := c.opts.MinimumRetryInterval
if minimumRetryInterval == 0 {
minimumRetryInterval = DefaultMinimumRetryInterval
}
// If we're being rate limited AND have stale cache, return it immediately
// This avoids blocking all goroutines when etcd has long timeout
if !lastRefresh.IsZero() && time.Since(lastRefresh) < minimumRetryInterval && len(cp) > 0 {
c.RUnlock()
// Return stale cache even if expired
return cp, nil
}
// unlock the read lock before potentially blocking operations
c.RUnlock()
@@ -175,8 +183,18 @@ func (c *cache) get(service string) ([]*registry.Service, error) {
// Use singleflight to deduplicate concurrent requests
val, err, _ := c.sg.Do(service, func() (interface{}, error) {
// Inside singleflight - only one goroutine executes this
// Apply rate limiting to prevent excessive registry calls
if !lastRefresh.IsZero() && time.Since(lastRefresh) < minimumRetryInterval {
// Re-check rate limiting inside singleflight
// (in case another goroutine just completed a refresh)
c.RLock()
currentLastRefresh := c.lastRefreshAttempt[service]
currentMinimumRetryInterval := c.opts.MinimumRetryInterval
if currentMinimumRetryInterval == 0 {
currentMinimumRetryInterval = DefaultMinimumRetryInterval
}
c.RUnlock()
if !currentLastRefresh.IsZero() && time.Since(currentLastRefresh) < currentMinimumRetryInterval {
// We're being rate limited
// Check if we have stale cache to return
c.RLock()