Skip to content

Commit 4f0bfc5

Browse files
committed
Merge commit '388a46b9c10b5653c71ac8840bcda0c91b59bce4' of https://github.com/apache/cassandra-java-driver into pull-upstream-4.18.1-v3
Upstream fix for CASSJAVA-7: Object reference in Micrometer metrics prevent GC from reclaiming Session instances
2 parents 6f9f7d5 + 388a46b commit 4f0bfc5

File tree

12 files changed

+54
-7
lines changed

12 files changed

+54
-7
lines changed

core/src/main/java/com/datastax/oss/driver/internal/core/metrics/AbstractMetricUpdater.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,4 @@ protected Timeout newTimeout() {
180180
expireAfter.toNanos(),
181181
TimeUnit.NANOSECONDS);
182182
}
183-
184-
protected abstract void clearMetrics();
185183
}

core/src/main/java/com/datastax/oss/driver/internal/core/metrics/DropwizardMetricUpdater.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public void updateTimer(
9191
}
9292

9393
@Override
94-
protected void clearMetrics() {
94+
public void clearMetrics() {
9595
for (MetricT metric : metrics.keySet()) {
9696
MetricId id = getMetricId(metric);
9797
registry.remove(id.getName());

core/src/main/java/com/datastax/oss/driver/internal/core/metrics/MetricUpdater.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,6 @@ default void markMeter(MetricT metric, @Nullable String profileName) {
4646
void updateTimer(MetricT metric, @Nullable String profileName, long duration, TimeUnit unit);
4747

4848
boolean isEnabled(MetricT metric, @Nullable String profileName);
49+
50+
void clearMetrics();
4951
}

core/src/main/java/com/datastax/oss/driver/internal/core/metrics/NoopNodeMetricUpdater.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,9 @@ public boolean isEnabled(NodeMetric metric, String profileName) {
5353
// since methods don't do anything, return false
5454
return false;
5555
}
56+
57+
@Override
58+
public void clearMetrics() {
59+
// nothing to do
60+
}
5661
}

core/src/main/java/com/datastax/oss/driver/internal/core/metrics/NoopSessionMetricUpdater.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,7 @@ public boolean isEnabled(SessionMetric metric, String profileName) {
5353
// since methods don't do anything, return false
5454
return false;
5555
}
56+
57+
@Override
58+
public void clearMetrics() {}
5659
}

core/src/main/java/com/datastax/oss/driver/internal/core/session/DefaultSession.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.datastax.oss.driver.internal.core.channel.DriverChannel;
4242
import com.datastax.oss.driver.internal.core.context.InternalDriverContext;
4343
import com.datastax.oss.driver.internal.core.context.LifecycleListener;
44+
import com.datastax.oss.driver.internal.core.metadata.DefaultNode;
4445
import com.datastax.oss.driver.internal.core.metadata.MetadataManager;
4546
import com.datastax.oss.driver.internal.core.metadata.MetadataManager.RefreshSchemaResult;
4647
import com.datastax.oss.driver.internal.core.metadata.NodeStateEvent;
@@ -568,6 +569,13 @@ private void close() {
568569

569570
closePolicies();
570571

572+
// clear metrics to prevent memory leak
573+
for (Node n : metadataManager.getMetadata().getNodes().values()) {
574+
((DefaultNode) n).getMetricUpdater().clearMetrics();
575+
}
576+
577+
DefaultSession.this.metricUpdater.clearMetrics();
578+
571579
List<CompletionStage<Void>> childrenCloseStages = new ArrayList<>();
572580
for (AsyncAutoCloseable closeable : internalComponentsToClose()) {
573581
childrenCloseStages.add(closeable.closeAsync());
@@ -587,6 +595,13 @@ private void forceClose() {
587595
logPrefix,
588596
(closeWasCalled ? "" : "not "));
589597

598+
// clear metrics to prevent memory leak
599+
for (Node n : metadataManager.getMetadata().getNodes().values()) {
600+
((DefaultNode) n).getMetricUpdater().clearMetrics();
601+
}
602+
603+
DefaultSession.this.metricUpdater.clearMetrics();
604+
590605
if (closeWasCalled) {
591606
// onChildrenClosed has already been scheduled
592607
for (AsyncAutoCloseable closeable : internalComponentsToClose()) {

integration-tests/src/test/java/com/datastax/oss/driver/core/metrics/DropwizardMetricsIT.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,12 @@ protected void assertNodeMetricsNotEvicted(CqlSession session, Node node) {
198198
}
199199
}
200200

201+
@Override
202+
protected void assertMetricsNotPresent(Object registry) {
203+
MetricRegistry dropwizardRegistry = (MetricRegistry) registry;
204+
assertThat(dropwizardRegistry.getMetrics()).isEmpty();
205+
}
206+
201207
@Override
202208
protected void assertNodeMetricsEvicted(CqlSession session, Node node) {
203209
InternalDriverContext context = (InternalDriverContext) session.getContext();

integration-tests/src/test/java/com/datastax/oss/driver/core/metrics/MetricsITBase.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,10 @@ public void resetSimulacron() {
8383

8484
@Test
8585
@UseDataProvider("descriptorsAndPrefixes")
86-
public void should_expose_metrics_if_enabled(Class<?> metricIdGenerator, String prefix) {
86+
public void should_expose_metrics_if_enabled_and_clear_metrics_if_closed(
87+
Class<?> metricIdGenerator, String prefix) {
8788

89+
Object registry = newMetricRegistry();
8890
Assume.assumeFalse(
8991
"Cannot use metric tags with Dropwizard",
9092
metricIdGenerator.getSimpleName().contains("Tagging")
@@ -101,12 +103,14 @@ public void should_expose_metrics_if_enabled(Class<?> metricIdGenerator, String
101103
CqlSession.builder()
102104
.addContactEndPoints(simulacron().getContactPoints())
103105
.withConfigLoader(loader)
104-
.withMetricRegistry(newMetricRegistry())
106+
.withMetricRegistry(registry)
105107
.build()) {
106108

107109
session.prepare("irrelevant");
108110
queryAllNodes(session);
109111
assertMetricsPresent(session);
112+
} finally {
113+
assertMetricsNotPresent(registry);
110114
}
111115
}
112116

@@ -262,4 +266,6 @@ private DefaultNode findNode(CqlSession session, int id) {
262266
return (DefaultNode)
263267
session.getMetadata().findNode(address1).orElseThrow(IllegalStateException::new);
264268
}
269+
270+
protected abstract void assertMetricsNotPresent(Object registry);
265271
}

integration-tests/src/test/java/com/datastax/oss/driver/metrics/micrometer/MicrometerMetricsIT.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,12 @@ protected void assertNodeMetricsNotEvicted(CqlSession session, Node node) {
188188
}
189189
}
190190

191+
@Override
192+
protected void assertMetricsNotPresent(Object registry) {
193+
MeterRegistry micrometerRegistry = (MeterRegistry) registry;
194+
assertThat(micrometerRegistry.getMeters()).isEmpty();
195+
}
196+
191197
@Override
192198
protected void assertNodeMetricsEvicted(CqlSession session, Node node) {
193199
InternalDriverContext context = (InternalDriverContext) session.getContext();

integration-tests/src/test/java/com/datastax/oss/driver/metrics/microprofile/MicroProfileMetricsIT.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,12 @@ protected void assertNodeMetricsNotEvicted(CqlSession session, Node node) {
188188
}
189189
}
190190

191+
@Override
192+
protected void assertMetricsNotPresent(Object registry) {
193+
MetricRegistry metricRegistry = (MetricRegistry) registry;
194+
assertThat(metricRegistry.getMetrics()).isEmpty();
195+
}
196+
191197
@Override
192198
protected void assertNodeMetricsEvicted(CqlSession session, Node node) {
193199
InternalDriverContext context = (InternalDriverContext) session.getContext();

0 commit comments

Comments
 (0)