You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/05/06 06:30:54 UTC
[GitHub] [incubator-ratis] avijayanhwx commented on a change in pull request #76: RATIS-845. Fix memory leak of RaftServerImpl for no unregister from reporter
avijayanhwx commented on a change in pull request #76:
URL: https://github.com/apache/incubator-ratis/pull/76#discussion_r420569786
##########
File path: ratis-metrics/src/main/java/org/apache/ratis/metrics/MetricsReporting.java
##########
@@ -34,17 +34,41 @@ private MetricsReporting() {
}
public static Consumer<RatisMetricRegistry> consoleReporter(TimeDuration rate) {
- return ratisMetricRegistry -> ConsoleReporter.forRegistry(ratisMetricRegistry.getDropWizardMetricRegistry())
- .convertRatesTo(TimeUnit.SECONDS).convertDurationsTo(TimeUnit.MILLISECONDS).build()
- .start(rate.getDuration(), rate.getUnit());
+ return ratisMetricRegistry -> {
+ ConsoleReporter reporter = ConsoleReporter.forRegistry(ratisMetricRegistry.getDropWizardMetricRegistry())
+ .convertRatesTo(TimeUnit.SECONDS).convertDurationsTo(TimeUnit.MILLISECONDS).build();
+ reporter.start(rate.getDuration(), rate.getUnit());
+ ratisMetricRegistry.setConsoleReporter(reporter);
+ };
+ }
+
+ public static Consumer<RatisMetricRegistry> stopConsoleReporter() {
Review comment:
Thanks @runzhiwang.
##########
File path: ratis-metrics/src/main/java/org/apache/ratis/metrics/MetricsReporting.java
##########
@@ -34,17 +34,41 @@ private MetricsReporting() {
}
public static Consumer<RatisMetricRegistry> consoleReporter(TimeDuration rate) {
- return ratisMetricRegistry -> ConsoleReporter.forRegistry(ratisMetricRegistry.getDropWizardMetricRegistry())
- .convertRatesTo(TimeUnit.SECONDS).convertDurationsTo(TimeUnit.MILLISECONDS).build()
- .start(rate.getDuration(), rate.getUnit());
+ return ratisMetricRegistry -> {
+ ConsoleReporter reporter = ConsoleReporter.forRegistry(ratisMetricRegistry.getDropWizardMetricRegistry())
+ .convertRatesTo(TimeUnit.SECONDS).convertDurationsTo(TimeUnit.MILLISECONDS).build();
+ reporter.start(rate.getDuration(), rate.getUnit());
+ ratisMetricRegistry.setConsoleReporter(reporter);
Review comment:
Can you tell me why we need this change? Instead of returning a function that starts the console reporter for a given RatisMetricRegistry, why are we starting and using a setter?
`ratisMetricRegistry.setConsoleReporter(reporter);`
##########
File path: ratis-metrics/src/main/java/org/apache/ratis/metrics/MetricsReporting.java
##########
@@ -34,17 +34,41 @@ private MetricsReporting() {
}
public static Consumer<RatisMetricRegistry> consoleReporter(TimeDuration rate) {
- return ratisMetricRegistry -> ConsoleReporter.forRegistry(ratisMetricRegistry.getDropWizardMetricRegistry())
- .convertRatesTo(TimeUnit.SECONDS).convertDurationsTo(TimeUnit.MILLISECONDS).build()
- .start(rate.getDuration(), rate.getUnit());
+ return ratisMetricRegistry -> {
+ ConsoleReporter reporter = ConsoleReporter.forRegistry(ratisMetricRegistry.getDropWizardMetricRegistry())
+ .convertRatesTo(TimeUnit.SECONDS).convertDurationsTo(TimeUnit.MILLISECONDS).build();
+ reporter.start(rate.getDuration(), rate.getUnit());
+ ratisMetricRegistry.setConsoleReporter(reporter);
+ };
+ }
+
+ public static Consumer<RatisMetricRegistry> stopConsoleReporter() {
+ return ratisMetricRegistry -> {
+ ConsoleReporter reporter = ratisMetricRegistry.getConsoleReporter();
+ if (reporter != null) {
+ reporter.close();
+ }
+ };
}
public static Consumer<RatisMetricRegistry> jmxReporter() {
return registry -> {
Builder builder =
JmxReporter.forRegistry(registry.getDropWizardMetricRegistry());
builder.inDomain(registry.getMetricRegistryInfo().getApplicationName());
- builder.build().start();
+ JmxReporter reporter = builder.build();
+ reporter.start();
+
+ registry.setJmxReporter(reporter);
Review comment:
Same here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org