You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/12/05 02:18:16 UTC

[GitHub] [bookkeeper] hangc0276 commented on a diff in pull request #3677: Make `jvm_memory_direct_bytes_used` metrics compatible with jdk8.

hangc0276 commented on code in PR #3677:
URL: https://github.com/apache/bookkeeper/pull/3677#discussion_r1039087659


##########
stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java:
##########
@@ -211,14 +214,32 @@ private void registerMetrics(Collector collector) {
         }
     }
 
-
     private static final Logger log = LoggerFactory.getLogger(PrometheusMetricsProvider.class);
 
-    private static final Optional<BufferPoolMXBean> poolMxBeanOp;
+    /*
+     * Try to get Netty counter of used direct memory. This will be correct, unlike the JVM values.
+     */
+    private static AtomicLong directMemoryUsage;
+    private static Optional<BufferPoolMXBean> poolMxBeanOp;
+    private static Supplier<Long> getDirectMemoryUsage;
 
     static {
-        List<BufferPoolMXBean> platformMXBeans = ManagementFactory.getPlatformMXBeans(BufferPoolMXBean.class);
-        poolMxBeanOp = platformMXBeans.stream()
-                .filter(bufferPoolMXBean -> bufferPoolMXBean.getName().equals("direct")).findAny();
+        if (PlatformDependent.useDirectBufferNoCleaner()) {
+            AtomicLong tmpDirectMemoryUsage = null;
+            try {
+                Field field = PlatformDependent.class.getDeclaredField("DIRECT_MEMORY_COUNTER");
+                field.setAccessible(true);
+                tmpDirectMemoryUsage = (AtomicLong) field.get(null);
+            } catch (Throwable t) {
+                log.warn("Failed to access netty DIRECT_MEMORY_COUNTER field {}", t.getMessage());
+            }
+            directMemoryUsage = tmpDirectMemoryUsage;
+            getDirectMemoryUsage = () -> directMemoryUsage.get();
+        } else {
+            List<BufferPoolMXBean> platformMXBeans = ManagementFactory.getPlatformMXBeans(BufferPoolMXBean.class);
+            poolMxBeanOp = platformMXBeans.stream()
+                    .filter(bufferPoolMXBean -> bufferPoolMXBean.getName().equals("direct")).findAny();
+            getDirectMemoryUsage = () -> poolMxBeanOp.get().getMemoryUsed();

Review Comment:
   We need to check whether poolMxBeanOp is present before get



##########
stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java:
##########
@@ -211,14 +214,32 @@ private void registerMetrics(Collector collector) {
         }
     }
 
-
     private static final Logger log = LoggerFactory.getLogger(PrometheusMetricsProvider.class);
 
-    private static final Optional<BufferPoolMXBean> poolMxBeanOp;
+    /*
+     * Try to get Netty counter of used direct memory. This will be correct, unlike the JVM values.
+     */
+    private static AtomicLong directMemoryUsage;
+    private static Optional<BufferPoolMXBean> poolMxBeanOp;
+    private static Supplier<Long> getDirectMemoryUsage;
 
     static {
-        List<BufferPoolMXBean> platformMXBeans = ManagementFactory.getPlatformMXBeans(BufferPoolMXBean.class);
-        poolMxBeanOp = platformMXBeans.stream()
-                .filter(bufferPoolMXBean -> bufferPoolMXBean.getName().equals("direct")).findAny();
+        if (PlatformDependent.useDirectBufferNoCleaner()) {
+            AtomicLong tmpDirectMemoryUsage = null;
+            try {
+                Field field = PlatformDependent.class.getDeclaredField("DIRECT_MEMORY_COUNTER");
+                field.setAccessible(true);
+                tmpDirectMemoryUsage = (AtomicLong) field.get(null);
+            } catch (Throwable t) {
+                log.warn("Failed to access netty DIRECT_MEMORY_COUNTER field {}", t.getMessage());
+            }
+            directMemoryUsage = tmpDirectMemoryUsage;
+            getDirectMemoryUsage = () -> directMemoryUsage.get();

Review Comment:
   We need to check whether the `directMemoryUsage` is `null` before get



##########
stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java:
##########
@@ -211,14 +214,32 @@ private void registerMetrics(Collector collector) {
         }
     }
 
-
     private static final Logger log = LoggerFactory.getLogger(PrometheusMetricsProvider.class);
 
-    private static final Optional<BufferPoolMXBean> poolMxBeanOp;
+    /*
+     * Try to get Netty counter of used direct memory. This will be correct, unlike the JVM values.
+     */
+    private static AtomicLong directMemoryUsage;
+    private static Optional<BufferPoolMXBean> poolMxBeanOp;
+    private static Supplier<Long> getDirectMemoryUsage;

Review Comment:
   Can it be `final`?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org