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

Reply via email to