adutra commented on code in PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#discussion_r1532801560
########## metrics/micrometer/src/main/java/com/datastax/oss/driver/internal/metrics/micrometer/MicrometerNodeMetricUpdater.java: ########## @@ -31,13 +31,18 @@ import io.micrometer.core.instrument.Timer; import java.time.Duration; import java.util.Set; +import java.util.function.Supplier; import net.jcip.annotations.ThreadSafe; @ThreadSafe public class MicrometerNodeMetricUpdater extends MicrometerMetricUpdater<NodeMetric> implements NodeMetricUpdater { private final Node node; + private final Supplier<Number> openConnectionsSupplier; Review Comment: Oooh I see, we are keeping a strong ref here to compensate for the weak ref there. Smart :-) But I agree with you that we need to better document the intent. Not only IDEs will complain (mine is complaining), but it won't be long until someone refactors this and removes the final fields, thus silently re-introducing the memory leak. A simple trick to calm down IDEs is to move all the calls to `initializeGauge` to a separate method: ```java public MicrometerNodeMetricUpdater( Node node, InternalDriverContext context, Set<NodeMetric> enabledMetrics, MeterRegistry registry) { super(context, enabledMetrics, registry); this.node = node; // Keep a strong reference to all gauge suppliers, otherwise they will be GC'ed too soon openConnectionsSupplier = node::getOpenConnections; streamSupplier = () -> availableStreamIds(node); inFlightRequestsSupplier = () -> inFlightRequests(node); orphanedStreamIdsSupplier = () -> orphanedStreamIds(node); initMetrics(); } private void initMetrics() { DriverExecutionProfile profile = context.getConfig().getDefaultProfile(); initializeGauge(DefaultNodeMetric.OPEN_CONNECTIONS, profile, openConnectionsSupplier); initializeGauge(DefaultNodeMetric.AVAILABLE_STREAMS, profile, streamSupplier); initializeGauge(DefaultNodeMetric.IN_FLIGHT, profile, inFlightRequestsSupplier); initializeGauge(DefaultNodeMetric.ORPHANED_STREAMS, profile, orphanedStreamIdsSupplier); // ... ``` -- 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: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org