chirag-wadhwa5 commented on code in PR #19500:
URL: https://github.com/apache/kafka/pull/19500#discussion_r2049318313
##########
server/src/main/java/org/apache/kafka/server/share/session/ShareSessionCache.java:
##########
@@ -140,43 +125,19 @@ public synchronized void touch(ShareSession session, long
now) {
}
}
- /**
- * Try to evict an entry from the session cache.
- * <p>
- * A proposed new element A may evict an existing element B if:
- * B is considered "stale" because it has been inactive for a long time.
- *
- * @param now The current time in milliseconds.
- * @return True if an entry was evicted; false otherwise.
- */
- public synchronized boolean tryEvict(long now) {
- // Try to evict an entry which is stale.
- Map.Entry<LastUsedKey, ShareSession> lastUsedEntry =
lastUsed.firstEntry();
- if (lastUsedEntry == null) {
- return false;
- } else if (now - lastUsedEntry.getKey().lastUsedMs() > evictionMs) {
- ShareSession session = lastUsedEntry.getValue();
- remove(session);
- evictionsMeter.mark();
Review Comment:
Thanks for the review. There are no evictions but sessions are still dropped
on client disconnections. I believe `ShareSessionsDroppedPerSec` would be a
better name. We can target the question of where it will go in the PR that
introduces the client connection drop logic. Will update that PR once this one
gets merged. But yeah, that shouldn't go in the remove method because I don't
think we need this metric to also include the sessions dropped due to Final
Share Fetch request.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
