Skip to content

Commit 4838a57

Browse files
committed
address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
1 parent 24e330e commit 4838a57

1 file changed

Lines changed: 38 additions & 40 deletions

File tree

opensearch/src/main/java/org/opensearch/sql/opensearch/monitor/GCedMemoryUsage.java

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@
88
import com.sun.management.GarbageCollectionNotificationInfo;
99
import java.lang.management.GarbageCollectorMXBean;
1010
import java.lang.management.ManagementFactory;
11-
import java.util.Arrays;
1211
import java.util.List;
12+
import java.util.Locale;
1313
import java.util.Map;
14-
import java.util.Set;
1514
import java.util.concurrent.atomic.AtomicLong;
1615
import javax.management.Notification;
1716
import javax.management.NotificationEmitter;
@@ -23,6 +22,8 @@
2322
/** Get memory usage from GC notification listener, which is used in Calcite engine. */
2423
public class GCedMemoryUsage implements MemoryUsage {
2524
private static final Logger LOG = LogManager.getLogger();
25+
private static final List<String> OLD_GEN_GC_ACTION_KEYWORDS =
26+
List.of("major", "concurrent", "old", "full", "marksweep");
2627

2728
private GCedMemoryUsage() {
2829
registerGCListener();
@@ -57,53 +58,50 @@ public void setUsage(long value) {
5758
private void registerGCListener() {
5859
List<GarbageCollectorMXBean> gcBeans = ManagementFactory.getGarbageCollectorMXBeans();
5960
for (GarbageCollectorMXBean gcBean : gcBeans) {
60-
if (gcBean instanceof NotificationEmitter) {
61+
if (gcBean instanceof NotificationEmitter && isOldGenGc(gcBean.getName())) {
62+
LOG.info("{} listener registered for memory usage monitor.", gcBean.getName());
6163
NotificationEmitter emitter = (NotificationEmitter) gcBean;
62-
emitter.addNotificationListener(new OldGenGCListener(), null, null);
64+
emitter.addNotificationListener(
65+
new OldGenGCListener(),
66+
notification -> {
67+
if (!notification
68+
.getType()
69+
.equals(GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION)) {
70+
return false;
71+
}
72+
CompositeData cd = (CompositeData) notification.getUserData();
73+
GarbageCollectionNotificationInfo info = GarbageCollectionNotificationInfo.from(cd);
74+
return isOldGenGc(info.getGcAction());
75+
},
76+
null);
6377
}
6478
}
6579
}
6680

81+
private boolean isOldGenGc(String gcKeyword) {
82+
String keyword = gcKeyword.toLowerCase(Locale.ROOT);
83+
return OLD_GEN_GC_ACTION_KEYWORDS.stream().anyMatch(keyword::contains);
84+
}
85+
6786
private static class OldGenGCListener implements NotificationListener {
6887
@Override
6988
public void handleNotification(Notification notification, Object handback) {
70-
if (notification
71-
.getType()
72-
.equals(GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION)) {
73-
CompositeData cd = (CompositeData) notification.getUserData();
74-
GarbageCollectionNotificationInfo info = GarbageCollectionNotificationInfo.from(cd);
75-
76-
String gcAction = info.getGcAction();
77-
if (gcAction.contains("major")
78-
|| gcAction.contains("concurrent")
79-
|| gcAction.contains("old")
80-
|| gcAction.contains("full")) {
81-
Map<String, java.lang.management.MemoryUsage> memoryUsageAfterGc =
82-
info.getGcInfo().getMemoryUsageAfterGc();
83-
String possibleOldGenKey = findOldGenKey(memoryUsageAfterGc.keySet());
84-
if (possibleOldGenKey != null) {
85-
java.lang.management.MemoryUsage oldGenUsage =
86-
memoryUsageAfterGc.get(possibleOldGenKey);
87-
getInstance().setUsage(oldGenUsage.getUsed());
88-
if (LOG.isDebugEnabled()) {
89-
LOG.debug(
90-
"Old Gen GC detected, memory usage after GC is {} bytes.", getInstance().usage());
91-
}
92-
}
93-
}
94-
}
95-
}
96-
97-
private String findOldGenKey(Set<String> memoryPoolNames) {
98-
LOG.info("Memory pool names: {}", memoryPoolNames);
99-
List<String> possibleOldGenKeys =
100-
Arrays.asList("G1 Old Gen", "PS Old Gen", "CMS Old Gen", "Tenured Gen", "Old Gen");
101-
for (String key : possibleOldGenKeys) {
102-
if (memoryPoolNames.contains(key)) {
103-
return key;
104-
}
89+
CompositeData cd = (CompositeData) notification.getUserData();
90+
GarbageCollectionNotificationInfo info = GarbageCollectionNotificationInfo.from(cd);
91+
Map<String, java.lang.management.MemoryUsage> memoryUsageAfterGc =
92+
info.getGcInfo().getMemoryUsageAfterGc();
93+
// Skip Metaspace and CodeHeap spaces which the GC scope is out of stack GC.
94+
long totalStackUsed =
95+
memoryUsageAfterGc.entrySet().stream()
96+
.filter(
97+
entry ->
98+
!entry.getKey().equals("Metaspace") && !entry.getKey().startsWith("CodeHeap"))
99+
.mapToLong(entry -> entry.getValue().getUsed())
100+
.sum();
101+
getInstance().setUsage(totalStackUsed);
102+
if (LOG.isDebugEnabled()) {
103+
LOG.debug("Old Gen GC detected, memory usage after GC is {} bytes.", totalStackUsed);
105104
}
106-
return null;
107105
}
108106
}
109107
}

0 commit comments

Comments
 (0)