[incubator-gobblin] branch master updated: [GOBBLIN-1029] Maintain the last GC stats to accurately report the difference in each interval[]

[su...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

suvasude pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-gobblin.git


The following commit(s) were added to refs/heads/master by this push:
     new 3e21a1b  [GOBBLIN-1029] Maintain the last GC stats to accurately report the difference in each interval[]
3e21a1b is described below

commit 3e21a1b6b7a846e09f0d8cd6dd20297c178bf5b6
Author: sv2000 <su...@gmail.com>
AuthorDate: Thu Jan 16 11:15:42 2020 -0800

    [GOBBLIN-1029] Maintain the last GC stats to accurately report the difference in each interval[]
    
    Closes #2871 from sv2000/gcStatsBug
---
 .../cluster/ContainerHealthMetricsService.java     | 28 +++++++++++++++-------
 .../cluster/ContainerHealthMetricsServiceTest.java |  5 ++++
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetricsService.java b/gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetricsService.java
index d603bae..ecd6224 100644
--- a/gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetricsService.java
+++ b/gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetricsService.java
@@ -32,6 +32,7 @@ import com.sun.management.OperatingSystemMXBean;
 import com.typesafe.config.Config;
 
 import lombok.Data;
+import lombok.Getter;
 
 import org.apache.gobblin.metrics.ContextAwareGauge;
 import org.apache.gobblin.metrics.RootMetricContext;
@@ -72,6 +73,11 @@ public class ContainerHealthMetricsService extends AbstractScheduledService {
   private final MemoryMXBean memoryMXBean;
   private final List<GarbageCollectorMXBean> garbageCollectorMXBeans;
 
+  @Getter
+  private GcStats lastGcStats;
+  @Getter
+  private GcStats currentGcStats;
+
   //Heap stats
   AtomicDouble processCpuLoad = new AtomicDouble(0);
   AtomicDouble systemCpuLoad = new AtomicDouble(0);
@@ -98,6 +104,8 @@ public class ContainerHealthMetricsService extends AbstractScheduledService {
     this.operatingSystemMXBean = ManagementFactory.getPlatformMXBean(OperatingSystemMXBean.class);
     this.memoryMXBean = ManagementFactory.getMemoryMXBean();
     this.garbageCollectorMXBeans = ManagementFactory.getGarbageCollectorMXBeans();
+    this.lastGcStats = new GcStats();
+    this.currentGcStats = new GcStats();
 
     //Build all the gauges and register them with the metrics registry.
     List<ContextAwareGauge<Double>> systemMetrics = buildGaugeList();
@@ -133,15 +141,19 @@ public class ContainerHealthMetricsService extends AbstractScheduledService {
     this.freePhysicalMemSize.set(this.operatingSystemMXBean.getFreePhysicalMemorySize());
     this.processHeapUsedSize.set(this.memoryMXBean.getHeapMemoryUsage().getUsed());
 
-    GcStats gcStats = collectGcStats();
-    //Since GC Beans report accumulated counts/durations, we need to subtract the previous values to obtain the counts/durations
+    //Get the new GC stats
+    this.currentGcStats = collectGcStats();
+    // Since GC Beans report accumulated counts/durations, we need to subtract the previous values to obtain the counts/durations
     // since the last measurement time.
-    this.minorGcCount.set(gcStats.getMinorCount() - this.minorGcCount.get());
-    this.minorGcDuration.set(gcStats.getMinorDuration() - this.minorGcDuration.get());
-    this.majorGcCount.set(gcStats.getMajorCount() - this.majorGcCount.get());
-    this.majorGcDuration.set(gcStats.getMajorDuration() - this.majorGcDuration.get());
-    this.unknownGcCount.set(gcStats.getUnknownCount() - this.unknownGcCount.get());
-    this.unknownGcDuration.set(gcStats.getUnknownDuration() - this.unknownGcDuration.get());
+    this.minorGcCount.set(this.currentGcStats.getMinorCount() - this.lastGcStats.getMinorCount());
+    this.minorGcDuration.set(this.currentGcStats.getMinorDuration() - this.lastGcStats.getMinorDuration());
+    this.majorGcCount.set(this.currentGcStats.getMajorCount() - this.lastGcStats.getMajorCount());
+    this.majorGcDuration.set(this.currentGcStats.getMajorDuration() - this.lastGcStats.getMajorDuration());
+    this.unknownGcCount.set(this.currentGcStats.getUnknownCount() - this.lastGcStats.getUnknownCount());
+    this.unknownGcDuration.set(this.currentGcStats.getUnknownDuration() - this.lastGcStats.getUnknownDuration());
+
+    //Update last collected stats
+    this.lastGcStats = this.currentGcStats;
   }
 
   protected List<ContextAwareGauge<Double>> buildGaugeList() {
diff --git a/gobblin-cluster/src/test/java/org/apache/gobblin/cluster/ContainerHealthMetricsServiceTest.java b/gobblin-cluster/src/test/java/org/apache/gobblin/cluster/ContainerHealthMetricsServiceTest.java
index 66efe74..2181c4c 100644
--- a/gobblin-cluster/src/test/java/org/apache/gobblin/cluster/ContainerHealthMetricsServiceTest.java
+++ b/gobblin-cluster/src/test/java/org/apache/gobblin/cluster/ContainerHealthMetricsServiceTest.java
@@ -30,6 +30,9 @@ public class ContainerHealthMetricsServiceTest {
     Config config = ConfigFactory.empty();
     ContainerHealthMetricsService service = new ContainerHealthMetricsService(config);
     service.runOneIteration();
+    //Ensure lastGcStats updated after each iteration
+    Assert.assertTrue(service.getCurrentGcStats() == service.getLastGcStats());
+    ContainerHealthMetricsService.GcStats previousLastGcStats = service.getLastGcStats();
     Assert.assertTrue( service.minorGcCount.get() >= 0);
     Assert.assertTrue( service.minorGcDuration.get() >= 0);
     Assert.assertTrue( service.majorGcCount.get() >= 0);
@@ -39,6 +42,8 @@ public class ContainerHealthMetricsServiceTest {
     double processCpuTime1 = service.processCpuTime.get();
     Thread.sleep(10);
     service.runOneIteration();
+    Assert.assertTrue(service.getCurrentGcStats() == service.getLastGcStats());
+    Assert.assertTrue(service.getLastGcStats() != previousLastGcStats);
     double processCpuTime2 = service.processCpuTime.get();
     Assert.assertTrue( processCpuTime1 <= processCpuTime2);
     Assert.assertTrue( service.minorGcCount.get() >= 0);