[GitHub] [ratis] xichen01 commented on a diff in pull request #744: RATIS-1705. Fix metrics leak

via GitHub Tue, 01 Aug 2023 19:44:19 -0700


xichen01 commented on code in PR #744:
URL: https://github.com/apache/ratis/pull/744#discussion_r1281321262



##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/metrics/MessageMetrics.java:
##########
@@ -82,7 +82,7 @@ public void rpcCompleted(String metricNamePrefix){
   /**
    * Increments the count of RPCs received on the server.
    */
-  public void rpcReceived(String metricNamePrefix){
+  synchronized public void rpcReceived(String metricNamePrefix){

Review Comment:
   OK, I have put the test results below.  Looks like it is working, the result 
looks the same as just adding `synchronized`.
   
   ## Remove the `synchronized` of `rpcReceived` and **not change** the `inc`
   
   Execute python3 /root/multiupload.py several times and check the Metrics
   ```bash
   [root@VM-8-3-centos ~/community/ratis]$ ozone sh bucket create 
s3v/testbucket -l OBJECT_STORE
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
   
   [root@VM-8-3-centos ~/community/ratis]$ python3 /root/multiupload.py
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
   
   [root@VM-8-3-centos ~/community/ratis]$ python3 /root/multiupload.py
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_UNAVAILABLE_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_OK_received_executed",
   
   [root@VM-8-3-centos ~/community/ratis]$ python3 /root/multiupload.py
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_UNAVAILABLE_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_OK_received_executed",
   
   [root@VM-8-3-centos ~/community/ratis]$ python3 /root/multiupload.py
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_OK_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_UNAVAILABLE_received_executed",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-37972096E181->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-89346135893F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-9E61C6E57DB2->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-893A39BBDF52->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_OK_received_executed",
   [root@VM-8-3-centos ~/community/ratis]$
   ```
   
   ## Remove the `synchronized` of `rpcReceived` and modify the `inc` to:
   ```java
   public class MessageMetrics extends RatisMetrics {
   //...
     private void inc(String metricNamePrefix, Type t) {
       types.get(t)
               .computeIfAbsent(metricNamePrefix, prefix -> 
getRegistry().counter(prefix + t.getSuffix()))
               .inc();
       final Map<String, LongCounter> counters = types.get(t);
       LongCounter c = counters.get(metricNamePrefix);
       if (c == null) {
         synchronized (counters) {
           c = counters.computeIfAbsent(metricNamePrefix, prefix -> 
getRegistry().counter(prefix + t.getSuffix()));
         }
       }
       c.inc();
     }
   //...
   }
   ```
   
   Execute python3 /root/multiupload.py several times and check the Metrics
   ```bash
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
   [root@VM-8-3-centos ~/community/ratis]$ python3 /root/multiupload.py
   
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
       "name" : 
"ratis:name=ratis.client_message_metrics.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
   
   [root@VM-8-3-centos ~/community/ratis]$ python3 /root/multiupload.py
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
       "name" : 
"ratis:name=ratis.client_message_metrics.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
   
   [root@VM-8-3-centos ~/community/ratis]$ python3 /root/multiupload.py
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
       "name" : 
"ratis:name=ratis.client_message_metrics.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
   
   [root@VM-8-3-centos ~/community/ratis]$ python3 /root/multiupload.py
   [root@VM-8-3-centos ~/community/ratis]$ curl -s http://127.0.0.1:9878/jmx | 
grep client_message_metrics
       "name" : 
"ratis:name=ratis.client_message_metrics.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_ordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7.client-16E4510B255F->02ea2b77-f16b-4203-8bf8-fc4d2f1d4ff7_ratis.grpc.RaftClientProtocolService_unordered_started_total",
       "name" : 
"ratis:name=ratis.client_message_metrics.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9.client-0F0AC9B99210->c7bc58e7-0558-4c3a-9611-50d367b0b3e9_ratis.grpc.RaftClientProtocolService_ordered_started_total",
   [root@VM-8-3-centos ~/community/ratis]
   ```
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

[GitHub] [ratis] xichen01 commented on a diff in pull request #744: RATIS-1705. Fix metrics leak

Reply via email to