Skip to content

Commit 388a46b

Browse files
SiyaoIsHidingabsurdfarce
authored andcommitted
patch by Jane He; reviewed by Alexandre Dutra and Bret McGuire for CASSANDRA-19457
1 parent 6c48329 commit 388a46b

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
@@ -34,6 +34,7 @@
3434
import com.datastax.oss.driver.internal.core.channel.DriverChannel;
3535
import com.datastax.oss.driver.internal.core.context.InternalDriverContext;
3636
import com.datastax.oss.driver.internal.core.context.LifecycleListener;
37+
import com.datastax.oss.driver.internal.core.metadata.DefaultNode;
3738
import com.datastax.oss.driver.internal.core.metadata.MetadataManager;
3839
import com.datastax.oss.driver.internal.core.metadata.MetadataManager.RefreshSchemaResult;
3940
import com.datastax.oss.driver.internal.core.metadata.NodeStateEvent;
@@ -546,6 +547,13 @@ private void close() {
546547

547548
closePolicies();
548549

550+
// clear metrics to prevent memory leak
551+
for (Node n : metadataManager.getMetadata().getNodes().values()) {
552+
((DefaultNode) n).getMetricUpdater().clearMetrics();
553+
}
554+
555+
DefaultSession.this.metricUpdater.clearMetrics();
556+
549557
List<CompletionStage<Void>> childrenCloseStages = new ArrayList<>();
550558
for (AsyncAutoCloseable closeable : internalComponentsToClose()) {
551559
childrenCloseStages.add(closeable.closeAsync());
@@ -565,6 +573,13 @@ private void forceClose() {
565573
logPrefix,
566574
(closeWasCalled ? "" : "not "));
567575

576+
// clear metrics to prevent memory leak
577+
for (Node n : metadataManager.getMetadata().getNodes().values()) {
578+
((DefaultNode) n).getMetricUpdater().clearMetrics();
579+
}
580+
581+
DefaultSession.this.metricUpdater.clearMetrics();
582+
568583
if (closeWasCalled) {
569584
// onChildrenClosed has already been scheduled
570585
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
@@ -186,6 +186,12 @@ protected void assertNodeMetricsNotEvicted(CqlSession session, Node node) {
186186
}
187187
}
188188

189+
@Override
190+
protected void assertMetricsNotPresent(Object registry) {
191+
MeterRegistry micrometerRegistry = (MeterRegistry) registry;
192+
assertThat(micrometerRegistry.getMeters()).isEmpty();
193+
}
194+
189195
@Override
190196
protected void assertNodeMetricsEvicted(CqlSession session, Node node) {
191197
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)