You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/03/23 14:06:39 UTC
[GitHub] [geode] geetrawat opened a new pull request #6176: GEODE-9051: Added a feature to measure the Tenured heap consumption
geetrawat opened a new pull request #6176:
URL: https://github.com/apache/geode/pull/6176
This feature prints out the tenured heap in the logs after the garbage collection. It works with JDK 1.8 build 212 and above.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
--
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
[GitHub] [geode] DonalEvans commented on a change in pull request #6176: GEODE-9051: Added a feature to measure the Tenured heap consumption
Posted by GitBox <gi...@apache.org>.
DonalEvans commented on a change in pull request #6176:
URL: https://github.com/apache/geode/pull/6176#discussion_r599864914
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/control/HeapMemoryMonitor.java
##########
@@ -670,12 +673,31 @@ public void run() {
// Not using the information given by the notification in favor
// of constructing fresh information ourselves.
if (!testDisableMemoryUpdates) {
+ try {
+ String notifType = notification.getType();
+ if (notifType.equals(MemoryNotificationInfo.MEMORY_THRESHOLD_EXCEEDED) ||
+ notifType.equals(MemoryNotificationInfo.MEMORY_COLLECTION_THRESHOLD_EXCEEDED)) {
+ // retrieve the memory notification information
+ CompositeData cd = (CompositeData) notification.getUserData();
+ MemoryNotificationInfo info = MemoryNotificationInfo.from(cd);
+ MemoryUsage usage = info.getUsage();
+ long usedBytes = usage.getUsed();
+ logger.info(
+ "A tenured heap garbage collection has occurred. New tenured heap consumption: "
+ +
+ usedBytes);
+ }
+ } catch (Exception e) {
+ logger.info(
+ "An Exception occureed while attempting to print out tenured heap consumption", e);
Review comment:
Typo here, this should be "occurred". Also, it might be better to say "while attempting to log tenured..."
--
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
[GitHub] [geode] DonalEvans merged pull request #6176: GEODE-9051: Added a feature to measure the Tenured heap consumption
Posted by GitBox <gi...@apache.org>.
DonalEvans merged pull request #6176:
URL: https://github.com/apache/geode/pull/6176
--
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
[GitHub] [geode] DonalEvans commented on a change in pull request #6176: GEODE-9051: Added a feature to measure the Tenured heap consumption
Posted by GitBox <gi...@apache.org>.
DonalEvans commented on a change in pull request #6176:
URL: https://github.com/apache/geode/pull/6176#discussion_r604375409
##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/control/TenuredHeapConsumptionMonitorTest.java
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.control;
+
+import static java.lang.management.MemoryNotificationInfo.MEMORY_COLLECTION_THRESHOLD_EXCEEDED;
+import static java.lang.management.MemoryNotificationInfo.MEMORY_THRESHOLD_EXCEEDED;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.lang.management.MemoryNotificationInfo;
+import java.lang.management.MemoryUsage;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
+
+import javax.management.Notification;
+import javax.management.openmbean.CompositeData;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.Pool;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+
+public class TenuredHeapConsumptionMonitorTest {
+
+ private BiConsumer<String, Throwable> infoLogger;
+ private Function<CompositeData, MemoryNotificationInfo> memoryNotificationInfoFactory;
+ private Notification notification;
+
+
+ @Before
+ public void setUp() {
+ infoLogger = mock(BiConsumer.class);
+ memoryNotificationInfoFactory = mock(Function.class);
+
+
+ }
+
+ private void notificationType(String type) {
+ notification = new Notification(type, this, 1);
+ ClientCache clientCache;
+ clientCache = new ClientCacheFactory().create();
+ GemFireCacheImpl gfc = (GemFireCacheImpl) clientCache;
+ Pool defPool = gfc.getDefaultPool();
+ MemoryUsage usage = new MemoryUsage(50, 50, 50, 100);
+ MemoryNotificationInfo memoryNotificationInfo =
+ new MemoryNotificationInfo(defPool.getName(), usage, 1);
+ when(memoryNotificationInfoFactory.apply(any())).thenReturn(memoryNotificationInfo);
+ TenuredHeapConsumptionMonitor monitor =
+ new TenuredHeapConsumptionMonitor(infoLogger, memoryNotificationInfoFactory);
+
+ monitor.checkTenuredHeapConsumption(notification);
+
+ }
+
+ @Test
+ public void assertIfTenuredGCLogMessageIsPrintedAfterGCAndWhenMemoryThresholdExceeds() {
+ notificationType(MEMORY_THRESHOLD_EXCEEDED);
Review comment:
With the changes to the `setUp()` method, this would become:
```suggestion
when(notification.getType()).thenReturn(MEMORY_THRESHOLD_EXCEEDED);
monitor.checkTenuredHeapConsumption(notification);
```
##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/control/TenuredHeapConsumptionMonitorTest.java
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.control;
+
+import static java.lang.management.MemoryNotificationInfo.MEMORY_COLLECTION_THRESHOLD_EXCEEDED;
+import static java.lang.management.MemoryNotificationInfo.MEMORY_THRESHOLD_EXCEEDED;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.lang.management.MemoryNotificationInfo;
+import java.lang.management.MemoryUsage;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
+
+import javax.management.Notification;
+import javax.management.openmbean.CompositeData;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.Pool;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+
+public class TenuredHeapConsumptionMonitorTest {
+
+ private BiConsumer<String, Throwable> infoLogger;
+ private Function<CompositeData, MemoryNotificationInfo> memoryNotificationInfoFactory;
+ private Notification notification;
+
+
+ @Before
+ public void setUp() {
+ infoLogger = mock(BiConsumer.class);
+ memoryNotificationInfoFactory = mock(Function.class);
+
+
+ }
+
+ private void notificationType(String type) {
+ notification = new Notification(type, this, 1);
+ ClientCache clientCache;
+ clientCache = new ClientCacheFactory().create();
+ GemFireCacheImpl gfc = (GemFireCacheImpl) clientCache;
+ Pool defPool = gfc.getDefaultPool();
+ MemoryUsage usage = new MemoryUsage(50, 50, 50, 100);
+ MemoryNotificationInfo memoryNotificationInfo =
+ new MemoryNotificationInfo(defPool.getName(), usage, 1);
+ when(memoryNotificationInfoFactory.apply(any())).thenReturn(memoryNotificationInfo);
+ TenuredHeapConsumptionMonitor monitor =
+ new TenuredHeapConsumptionMonitor(infoLogger, memoryNotificationInfoFactory);
+
+ monitor.checkTenuredHeapConsumption(notification);
+
+ }
+
+ @Test
+ public void assertIfTenuredGCLogMessageIsPrintedAfterGCAndWhenMemoryThresholdExceeds() {
+ notificationType(MEMORY_THRESHOLD_EXCEEDED);
+ verify(infoLogger).accept(
+ eq("A tenured heap garbage collection has occurred. New tenured heap consumption: 50"),
+ isNull());
Review comment:
If the `used` field is introduced, this verification doesn't need to use a hard-coded value but can instead be:
```suggestion
verify(infoLogger).accept(
eq("A tenured heap garbage collection has occurred. New tenured heap consumption: "
+ used),
isNull());
```
##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/control/TenuredHeapConsumptionMonitorTest.java
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.control;
+
+import static java.lang.management.MemoryNotificationInfo.MEMORY_COLLECTION_THRESHOLD_EXCEEDED;
+import static java.lang.management.MemoryNotificationInfo.MEMORY_THRESHOLD_EXCEEDED;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.lang.management.MemoryNotificationInfo;
+import java.lang.management.MemoryUsage;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
+
+import javax.management.Notification;
+import javax.management.openmbean.CompositeData;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.Pool;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+
+public class TenuredHeapConsumptionMonitorTest {
+
+ private BiConsumer<String, Throwable> infoLogger;
+ private Function<CompositeData, MemoryNotificationInfo> memoryNotificationInfoFactory;
+ private Notification notification;
+
+
+ @Before
+ public void setUp() {
+ infoLogger = mock(BiConsumer.class);
+ memoryNotificationInfoFactory = mock(Function.class);
+
+
+ }
Review comment:
By replacing the object creation in the `notificationType()` method with mocking and extracting it to the `setUp()` method, and adding `private final long used = 50;` and `private TenuredHeapConsumptionMonitor monitor;` fields, this class can be made a little more flexible and some new test cases added:
```suggestion
@Before
public void setUp() {
notification = mock(Notification.class);
MemoryUsage usage = mock(MemoryUsage.class);
MemoryNotificationInfo memoryNotificationInfo = mock(MemoryNotificationInfo.class);
when(memoryNotificationInfo.getUsage()).thenReturn(usage);
when(usage.getUsed()).thenReturn(used);
memoryNotificationInfoFactory = mock(Function.class);
when(memoryNotificationInfoFactory.apply(any())).thenReturn(memoryNotificationInfo);
infoLogger = mock(BiConsumer.class);
monitor = new TenuredHeapConsumptionMonitor(infoLogger, memoryNotificationInfoFactory);
}
```
##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/control/TenuredHeapConsumptionMonitorTest.java
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.control;
+
+import static java.lang.management.MemoryNotificationInfo.MEMORY_COLLECTION_THRESHOLD_EXCEEDED;
+import static java.lang.management.MemoryNotificationInfo.MEMORY_THRESHOLD_EXCEEDED;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.lang.management.MemoryNotificationInfo;
+import java.lang.management.MemoryUsage;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
+
+import javax.management.Notification;
+import javax.management.openmbean.CompositeData;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.Pool;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+
+public class TenuredHeapConsumptionMonitorTest {
+
+ private BiConsumer<String, Throwable> infoLogger;
+ private Function<CompositeData, MemoryNotificationInfo> memoryNotificationInfoFactory;
+ private Notification notification;
+
+
+ @Before
+ public void setUp() {
+ infoLogger = mock(BiConsumer.class);
+ memoryNotificationInfoFactory = mock(Function.class);
+
+
+ }
+
+ private void notificationType(String type) {
+ notification = new Notification(type, this, 1);
+ ClientCache clientCache;
+ clientCache = new ClientCacheFactory().create();
+ GemFireCacheImpl gfc = (GemFireCacheImpl) clientCache;
+ Pool defPool = gfc.getDefaultPool();
+ MemoryUsage usage = new MemoryUsage(50, 50, 50, 100);
+ MemoryNotificationInfo memoryNotificationInfo =
+ new MemoryNotificationInfo(defPool.getName(), usage, 1);
+ when(memoryNotificationInfoFactory.apply(any())).thenReturn(memoryNotificationInfo);
+ TenuredHeapConsumptionMonitor monitor =
+ new TenuredHeapConsumptionMonitor(infoLogger, memoryNotificationInfoFactory);
+
+ monitor.checkTenuredHeapConsumption(notification);
+
+ }
+
+ @Test
+ public void assertIfTenuredGCLogMessageIsPrintedAfterGCAndWhenMemoryThresholdExceeds() {
+ notificationType(MEMORY_THRESHOLD_EXCEEDED);
+ verify(infoLogger).accept(
+ eq("A tenured heap garbage collection has occurred. New tenured heap consumption: 50"),
+ isNull());
+
+
+ }
+
+ @Test
+ public void assertIfTenuredGCLogMessageIsPrintedAfterGCAndWhenMemoryCollectionThresholdExceeds() {
+ notificationType(MEMORY_COLLECTION_THRESHOLD_EXCEEDED);
+ verify(infoLogger).accept(
+ eq("A tenured heap garbage collection has occurred. New tenured heap consumption: 50"),
+ isNull());
+
+
Review comment:
With the changes to `setUp()` and the inclusion of the `used` field, this would become:
```suggestion
when(notification.getType()).thenReturn(MEMORY_COLLECTION_THRESHOLD_EXCEEDED);
monitor.checkTenuredHeapConsumption(notification);
verify(infoLogger).accept(
eq("A tenured heap garbage collection has occurred. New tenured heap consumption: "
+ used),
isNull());
```
##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/control/TenuredHeapConsumptionMonitorTest.java
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.control;
+
+import static java.lang.management.MemoryNotificationInfo.MEMORY_COLLECTION_THRESHOLD_EXCEEDED;
+import static java.lang.management.MemoryNotificationInfo.MEMORY_THRESHOLD_EXCEEDED;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.lang.management.MemoryNotificationInfo;
+import java.lang.management.MemoryUsage;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
+
+import javax.management.Notification;
+import javax.management.openmbean.CompositeData;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.Pool;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+
+public class TenuredHeapConsumptionMonitorTest {
+
+ private BiConsumer<String, Throwable> infoLogger;
+ private Function<CompositeData, MemoryNotificationInfo> memoryNotificationInfoFactory;
+ private Notification notification;
+
+
+ @Before
+ public void setUp() {
+ infoLogger = mock(BiConsumer.class);
+ memoryNotificationInfoFactory = mock(Function.class);
+
+
+ }
+
+ private void notificationType(String type) {
Review comment:
This method can be removed if the suggested changed to the `setUp()` method are made.
--
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