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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]