absurdfarce commented on code in PR #1916:
URL:
https://github.com/apache/cassandra-java-driver/pull/1916#discussion_r1532687869
##########
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:
This is really the heart of the fix. In the original implementation the
supplier was passed inline to Micrometer; there was no ref kept anywhere else.
The unexpected (at least to me) [strong reference used in the Gauge
impl](https://github.com/micrometer-metrics/micrometer/blob/v1.6.5/micrometer-core/src/main/java/io/micrometer/core/instrument/Gauge.java#L46-L61)
was the only thing preventing the Supplier from getting GC'd. And that was
entirely a side-effect of the constructor we were using. So if we then change
the Gauge implementation to not using strong references the only ref for the
Supplier is now the weak ref in the Gauge... which means it's eligible for GC
earlier than we'd like.
Creating a strong ref on the class avoids that issue; the key question is
where. It seemed reasonable to define it on the updater itself so that it
won't persist beyond the lifetime of this class.
I very much grant you this is all very indirect and non-intuitive. As a
result the impl is brittle and (at a bare minimum) needs some additional
commentary to explain what's going on.
--
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]
