ignite-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [ignite] alex-plekhanov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
Date Thu, 12 Sep 2019 07:13:04 GMT
alex-plekhanov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
URL: https://github.com/apache/ignite/pull/6845#discussion_r323587227
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/GridMetricManager.java
 ##########
 @@ -265,45 +378,116 @@ public MetricRegistry registry(String name) {
         return registries.computeIfAbsent(name, n -> {
             MetricRegistry mreg = new MetricRegistry(name, log);
 
-            notifyListeners(mreg, metricRegCreationLsnrs);
+            notifyListeners(mreg, metricRegCreationLsnrs, log);
 
             return mreg;
         });
     }
 
-    /** {@inheritDoc} */
-    @NotNull @Override public Iterator<MetricRegistry> iterator() {
-        return registries.values().iterator();
+    public <R extends MonitoringRow, D> void list(String name, String description,
+        Class<R> rowClazz, Supplier<ConcurrentMap<?, D>> data, Function<D,
R> rowFunc, Consumer<D> clearer) {
+
+        Supplier<MonitoringList<R>> listCreator = () -> list(name, () ->
new MonitoringListAdapter<>(name,
+            description,
+            rowClazz,
+            (MonitoringRowAttributeWalker<R>)walkers.get(rowClazz),
+            data.get(),
+            rowFunc));
+
+        //Create new instance of the list.
+        listCreator.get();
+
+        ctx.metric().addEnableListListener(listenOnlyEqual(name, identity(), n -> listCreator.get()));
 
 Review comment:
   I don't say that I didn't understand the code, I say that code is hard to understand because
it uses 5-6 nested lambdas. It's certainly not a pretty simple.  
   
   I don't propose to rename it to "list enabler", I say that "listener" is a bad abstraction
for this case.
   
   Perhaps code can be simplified in this way:
   1. Create maps for list creators and removers (name -> closure) instead of lists for
listeners 
   2. On enabling/disabling just call closure from this map by name.
   3. Get rid of as most nested lambdas as possible. Now it looks redundant. Ideally, for
list creation case there should be only two labdas: one for list creation closure, and one
for `computeIfAbsent` mapping function.

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


With regards,
Apache Git Services

Mime
View raw message