HDDS-7349. Flaky integration test have memory leak for RatisDropwizardExports#3858
HDDS-7349. Flaky integration test have memory leak for RatisDropwizardExports#3858adoroszlai merged 2 commits intoapache:masterfrom
Conversation
|
@nandakumar131 @ChenSammi Please review |
|
@szetszwo plz review |
|
@sumitagrawl , each datanode/om/scm creates its own MetricRegistries instead using global(); see https://issues.apache.org/jira/secure/attachment/13051264/MetricRegistries.patch |
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
@sumitagrawl , the change looks good. Some comments inlined; see also https://issues.apache.org/jira/secure/attachment/13051276/3858_review.patch
| public static void registerRatisMetricReporters( | ||
| Map<String, RatisDropwizardExports> ratisMetricsMap) { | ||
| Map<String, RatisDropwizardExports> ratisMetricsMap, | ||
| AtomicBoolean isStopped) { |
| private static void registerDropwizard(RatisMetricRegistry registry, | ||
| Map<String, RatisDropwizardExports> ratisMetricsMap) { | ||
| Map<String, RatisDropwizardExports> ratisMetricsMap, | ||
| AtomicBoolean isStopped) { |
| RatisDropwizardExports rde = new RatisDropwizardExports( | ||
| registry.getDropWizardMetricRegistry()); | ||
| CollectorRegistry.defaultRegistry.register(rde); | ||
| String name = registry.getMetricRegistryInfo().getName(); |
There was a problem hiding this comment.
Keep name declared after rde, i.e.
RatisDropwizardExports rde = new RatisDropwizardExports(
registry.getDropWizardMetricRegistry());
String name = registry.getMetricRegistryInfo().getName();| private static final int MSECS_PER_MINUTE = 60 * 1000; | ||
|
|
||
| private final boolean isSecurityEnabled; | ||
| private final AtomicBoolean isStopped = new AtomicBoolean(false); |
There was a problem hiding this comment.
Use omState and do not add isStopped.
| // This needs to be done before initializing Ratis. | ||
| RatisDropwizardExports. | ||
| registerRatisMetricReporters(ratisMetricsMap); | ||
| registerRatisMetricReporters(ratisMetricsMap, isStopped); |
There was a problem hiding this comment.
Use () -> omState == State.STOPPED.
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
+1 the change looks good.
@sumitagrawl , just found that I uploaded a wrong file. It was great that you have fixed everything already! Anyway, this is the correct file https://issues.apache.org/jira/secure/attachment/13051439/3858_review2.patch |
|
Thanks @sumitagrawl for the patch, @szetszwo for the review. |
3 participants
What changes were proposed in this pull request?
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7349
How was this patch tested?
Tested with flaky and taking heapdump for objects of RatisDropwizardExports referred by DNs ratisMetricsMap.