DonalEvans commented on a change in pull request #6427:
URL: https://github.com/apache/geode/pull/6427#discussion_r636302522
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveWriter.java
##########
@@ -508,6 +508,9 @@ public void sampled(long nanosTimeStamp,
List<ResourceInstance> resourceInstance
}
writeTimeStamp(nanosTimeStamp);
for (ResourceInstance ri : resourceInstances) {
+ if (!ri.getStatValuesNotified()) {
+ ri.setStatValuesNotified(true);
+ }
Review comment:
I think that the `if` is unnecessary here, since it's fine to call
`setStatValuesNotified(true)` if the value is already true.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/statistics/ValueMonitor.java
##########
@@ -98,6 +98,9 @@ protected void monitorStatistics(final long millisTimeStamp,
if (this.statistics.contains(resource.getStatistics())) {
ResourceType resourceType = resource.getResourceType();
StatisticDescriptor[] sds = resourceType.getStatisticDescriptors();
+ if (!resource.getStatValuesNotified()) {
+ resource.setStatValuesNotified(true);
+ }
Review comment:
I think that the `if` is unnecessary here, since it's fine to call
`setStatValuesNotified(true)` if the value is already true.
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/statistics/SampleCollectorTest.java
##########
@@ -210,7 +210,7 @@ public void testAddHandlerAfterSamples() {
ResourceInstance resourceInstance =
allocatedResourceInstanceInfo.getResourceInstance();
assertNotNull(resourceInstance);
assertEquals(0, resourceInstance.getId());
- assertEquals(0, resourceInstance.getUpdatedStats().length);
+ assertEquals(1, resourceInstance.getUpdatedStats().length);
Review comment:
Why has this value changed? It's not immediately obvious from the
changes why the behaviour here has been affected.
--
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
