Vladsz83 commented on a change in pull request #7446: IGNITE-12464 : Service 
metrics
URL: https://github.com/apache/ignite/pull/7446#discussion_r401133474
 
 

 ##########
 File path: 
modules/core/src/main/java/org/apache/ignite/internal/processors/service/IgniteServiceProcessor.java
 ##########
 @@ -1801,4 +1842,178 @@ private boolean enterBusy() {
     private void leaveBusy() {
         opsLock.readLock().unlock();
     }
+
+    /**
+     * Registers metrics to measure durations of service methods.
+     *
+     * @param srvc Service for invocations measurement.
+     * @param srvcName Name of {@code srvc}.
+     */
+    private void registerMetrics(Service srvc, String srvcName) {
+        
getInterfaces(srvc.getClass()).stream().map(Class::getMethods).flatMap(Arrays::stream)
 
 Review comment:
   Same here. You are right about usage of separate method. But the 
synchronization you suggest seems already relies in the logic above. I don’t 
think `IgniteServiceProcessor.undeploy()` and ` 
IgniteServiceProcessor.redeploy()` can happen concurrently but we should 
consider it can, ok. Let’s see:
   -    Metrics are erased by `IgniteServiceProcessor.unregisterMetrics()` in 2 
places (): `IgniteServiceProcessor.redeploy()` and 
`IgniteServiceProcessor.undeploy()`.
   -    Metrics are created by `IgniteServiceProcessor.registerMetrics()`  in 
one place: `IgniteServiceProcessor.redeploy()`.
   
   These calls are not any APIs, are private.
   
   All 3 calls covered with sync by service context collection given by same 
service id:
   ```
   Collection<ServiceContextImpl> ctxs …
   synchronized (ctxs) {
        unregisterMetrics(cfg.getName());
           }
   ```
   and
   ```
   Collection<ServiceContextImpl> ctxs …
               synchronized (ctxs) {
                   if (!ctxs.iterator().next().isCancelled())
                       registerMetrics(srvc, srvcCtx.name());
               } 
   ```
   
   I think I really missed the last part earlier. Thanks for the note. Isn’t it 
the synchronized block you suggest?

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


With regards,
Apache Git Services

Reply via email to