Copilot commented on code in PR #966:
URL: https://github.com/apache/dubbo-go-pixiu/pull/966#discussion_r3371159666
##########
pkg/filter/llm/proxy/filter.go:
##########
@@ -535,39 +559,35 @@ func (s *cooldownStore) sweepExpiredIfNeededLocked(now
time.Time, current cooldo
}
func (s *cooldownStore) sweepExpiredExceptLocked(now time.Time, current
cooldownKey) {
- for key, entry := range s.lastFailureByEndpoint {
+ for key, element := range s.lastFailureByEndpoint {
if key == current {
continue
}
+ entry := element.Value.(*cooldownEntry)
if now.Sub(entry.lastFailure) >= entry.ttl {
- delete(s.lastFailureByEndpoint, key)
+ s.removeLocked(key, element)
}
}
}
+// evictOldestIfFullLocked removes the least-recently-failed entry when the
+// store is at capacity, in O(1) via the front of the recency list. current is
+// the key about to be inserted; it is never in the list yet on this path, but
+// the guard keeps the invariant explicit so a future caller cannot evict the
+// entry it is in the middle of writing.
func (s *cooldownStore) evictOldestIfFullLocked(current cooldownKey) {
if len(s.lastFailureByEndpoint) < maxCooldownStoreEntries {
return
}
-
- var (
- oldestKey cooldownKey
- oldestEntry cooldownEntry
- found bool
- )
- for key, entry := range s.lastFailureByEndpoint {
- if key == current {
- continue
- }
- if !found || entry.lastFailure.Before(oldestEntry.lastFailure) {
- oldestKey = key
- oldestEntry = entry
- found = true
- }
+ oldest := s.recencyOrder.Front()
+ if oldest == nil {
+ return
}
- if found {
- delete(s.lastFailureByEndpoint, oldestKey)
+ oldestKey := oldest.Value.(*cooldownEntry).key
+ if oldestKey == current {
+ return
}
+ s.removeLocked(oldestKey, oldest)
Review Comment:
`evictOldestIfFullLocked` has a special-case `oldestKey == current` early
return. With the current call site (`markFailure` only calls this for brand-new
keys), `current` cannot be the front element’s key, so this branch is
effectively dead code and makes the eviction logic harder to reason about. It
also weakens the capacity invariant if a future refactor ever calls this with
an existing key or if map/list drift occurs. Consider removing the guard and
always evicting the front element.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
