[ https://issues.apache.org/jira/browse/GEODE-8520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17201055#comment-17201055 ]
ASF GitHub Bot commented on GEODE-8520: --------------------------------------- demery-pivotal commented on a change in pull request #5536: URL: https://github.com/apache/geode/pull/5536#discussion_r493853151 ########## File path: geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/GCStatsMonitor.java ########## @@ -39,69 +39,71 @@ * @see org.apache.geode.management.internal.beans.stats.MBeanStatsMonitor */ public class GCStatsMonitor extends MBeanStatsMonitor { + // this class uses these volatile variables to make sure reads are reading the latest values + // it is not using the parent's siteMap which is not volatile to keep the stats values. private volatile long collections = 0; private volatile long collectionTime = 0; - long getCollections() { - return collections; - } - - long getCollectionTime() { - return collectionTime; - } - public GCStatsMonitor(String name) { super(name); } - void decreasePrevValues(Map<String, Number> statsMap) { - collections -= statsMap.getOrDefault(StatsKey.VM_GC_STATS_COLLECTIONS, 0).longValue(); - collectionTime -= statsMap.getOrDefault(StatsKey.VM_GC_STATS_COLLECTION_TIME, 0).longValue(); - } - - void increaseStats(String name, Number value) { - if (name.equals(StatsKey.VM_GC_STATS_COLLECTIONS)) { - collections += value.longValue(); - return; - } - - if (name.equals(StatsKey.VM_GC_STATS_COLLECTION_TIME)) { - collectionTime += value.longValue(); - return; + @Override + // this will be called multiple times with different collector's stats + public void addStatisticsToMonitor(Statistics stats) { + monitor.addListener(this);// if already listener is added this will be a no-op + monitor.addStatistics(stats); + + // stats map should keep the sum of all the GC stats + StatisticDescriptor[] descriptors = stats.getType().getStatistics(); + for (StatisticDescriptor d : descriptors) { + String name = d.getName(); + Number value = stats.get(d); + if (name.equals(StatsKey.VM_GC_STATS_COLLECTIONS)) { + collections += value.longValue(); + } else if (name.equals(StatsKey.VM_GC_STATS_COLLECTION_TIME)) { + collectionTime += value.longValue(); + } } } @Override public Number getStatistic(String statName) { if (statName.equals(StatsKey.VM_GC_STATS_COLLECTIONS)) { - return getCollections(); + return collections; } if (statName.equals(StatsKey.VM_GC_STATS_COLLECTION_TIME)) { - return getCollectionTime(); + return collectionTime; } - return 0; } @Override public void handleNotification(StatisticsNotification notification) { - decreasePrevValues(statsMap); - + // sum up all the count and all the time in the stats included in this notification + long totalCount = 0; + long totalTime = 0; for (StatisticId statId : notification) { StatisticDescriptor descriptor = statId.getStatisticDescriptor(); String name = descriptor.getName(); Number value; - try { value = notification.getValue(statId); } catch (StatisticNotFoundException e) { value = 0; } - log(name, value); - increaseStats(name, value); - statsMap.put(name, value); + if (name.equals(StatsKey.VM_GC_STATS_COLLECTIONS)) { + totalCount += value.longValue(); + } + + else if (name.equals(StatsKey.VM_GC_STATS_COLLECTION_TIME)) { + totalTime += value.longValue(); + } } + + collections = totalCount; + collectionTime = totalTime; } Review comment: Let me correct my terminology a bit… I keep saying "GC type," and that's not the right distinction. What distinguishes one batch of GC stats from another is which GarbageCollectorMXBean the values come from. I think (but I'm not sure) that each GC bean reports about a particular memory space. So where I say "GC type," a better term might be "memory space". ---------------------------------------------------------------- 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: us...@infra.apache.org > GarbageCollectionCount metric is showing negative values > -------------------------------------------------------- > > Key: GEODE-8520 > URL: https://issues.apache.org/jira/browse/GEODE-8520 > Project: Geode > Issue Type: Bug > Components: management > Affects Versions: 1.13.0 > Reporter: Jinmei Liao > Priority: Major > Labels: GeodeOperationAPI, pull-request-available > > sometimes the memberMBean's garbageCollectionCount and garbageCollectionTime > metriics are showing negative values. -- This message was sent by Atlassian Jira (v8.3.4#803005)