eolivelli commented on code in PR #3951:
URL: https://github.com/apache/bookkeeper/pull/3951#discussion_r1188416551
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java:
##########
@@ -2058,6 +2060,32 @@ public long
getClientConnectBookieUnavailableLogThrottlingMs() {
return getLong(CLIENT_CONNECT_BOOKIE_UNAVAILABLE_LOG_THROTTLING,
5_000L);
}
+ /**
+ * Set the client max idle connections time for no any read-write
operations happened, in minutes.
+ * After this time, the connection will be closed.
+ *
+ * @param maxIdleConnectionsMinutes
+ * Default is -1 (disabled)
+ * @param unit
+ * @return client configuration.
+ */
+ public ClientConfiguration setClientMaxIdleConnectionsMinutes(
Review Comment:
why minutes and not "seconds" ?
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClientImpl.java:
##########
@@ -205,7 +228,7 @@ public PerChannelBookieClientPool lookupClient(BookieId
addr) {
}
PerChannelBookieClientPool newClientPool =
new DefaultPerChannelBookieClientPool(conf, this, addr,
numConnectionsPerBookie);
- PerChannelBookieClientPool oldClientPool =
channels.putIfAbsent(addr, newClientPool);
+ PerChannelBookieClientPool oldClientPool =
channels.asMap().putIfAbsent(addr, newClientPool);
Review Comment:
how much cost adds "asMap" ? is performing operations or is it no-op ?
if it is no-op then we could cache the result of "asMap" into a field.
if it performs operations, then we are adding work on the hot paths (anytime
we read/write) and we should find a better way to expire idle connections,
maybe not using a "Cache" but with a specific background task that scans the
map and removes idle connection
--
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]
