Copilot commented on code in PR #1221:
URL:
https://github.com/apache/skywalking-banyandb/pull/1221#discussion_r3621501693
##########
banyand/liaison/grpc/topk.go:
##########
@@ -80,12 +102,25 @@ func (t *topK) observe(key string, dur time.Duration) {
}
}
delete(t.slots, minKey)
- t.slots[key] = &topKSlot{key: key, count: minCount + 1, maxDur: dur}
+ t.slots[key] = &topKSlot{key: key, count: minCount + 1, maxDur: dur,
lastSeen: now, maxDurAt: now}
+}
+
+// purgeExpiredLocked drops every entry not observed within ttl. Call with the
lock held.
+func (t *topK) purgeExpiredLocked(now time.Time) {
+ if t.ttl <= 0 {
+ return
+ }
+ for k, s := range t.slots {
+ if now.Sub(s.lastSeen) > t.ttl {
+ delete(t.slots, k)
+ }
+ }
}
// snapshot returns the tracked entries ranked by frequency: (count desc,
maxDur desc,
-// key asc). The tracker is cumulative, so each dump reflects the hottest
queries since
-// process start. The full tie-break makes the order deterministic across
dumps.
+// key asc). Counts accumulate over an entry's lifetime, which the TTL bounds:
a dump
+// reflects the hottest queries within the TTL window, not since process
start. The full
+// tie-break makes the order deterministic across dumps.
Review Comment:
The snapshot() comment currently suggests the dump represents a "TTL
window", but TTL here is an inactivity-based expiry (keys are kept as long as
they are observed within ttl), and counts/maxDur remain cumulative while a key
stays active. Clarifying this avoids implying a sliding time window of
observations.
##########
banyand/liaison/grpc/bydbql.go:
##########
@@ -177,7 +177,7 @@ func (b *bydbQLService) Query(ctx context.Context, req
*bydbqlv1.QueryRequest) (
}
// topKDumper tracks the top re-parsed and slow queries and, on a supervised
-// goroutine, periodically logs the cumulative top-K. All methods are
nil-safe, so the
+// goroutine, periodically logs the top-K within each tracker's TTL window.
All methods are nil-safe, so the
// call sites need no guards when the top-K log is disabled (the dumper is
nil).
Review Comment:
This type-level comment describes the periodic dump as being "within each
tracker's TTL window", but TTL is implemented as inactivity-based expiry (based
on lastSeen), not a strict sliding time window of observations. Rewording
avoids misrepresenting what the logged counts/peaks mean.
--
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]
