mochengqian commented on code in PR #966:
URL: https://github.com/apache/dubbo-go-pixiu/pull/966#discussion_r3372107786
##########
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:
Good catch — fixed in 55f27151. `evictOldestIfFullLocked` is only ever
reached on the brand-new-key path (the existing-key branch returns early), so
`oldestKey == current` can never be true. And had it ever fired, it would have
skipped eviction while `markFailure` still `PushBack`s, pushing the store to
`maxCooldownStoreEntries + 1` — i.e. the guard weakened the very invariant it
claimed to protect. Dropped the guard and the now-unused `current` parameter;
capacity test still passes.
--
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]
