[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r326641197 ## File path: modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java ## @@ -0,0 +1,400 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.metric; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.CyclicBarrier; +import org.apache.ignite.IgniteCache; +import org.apache.ignite.IgniteCompute; +import org.apache.ignite.IgniteException; +import org.apache.ignite.cluster.ClusterNode; +import org.apache.ignite.compute.ComputeJob; +import org.apache.ignite.compute.ComputeJobResult; +import org.apache.ignite.compute.ComputeJobResultPolicy; +import org.apache.ignite.compute.ComputeTask; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.internal.IgniteEx; +import org.apache.ignite.lang.IgniteCallable; +import org.apache.ignite.lang.IgniteClosure; +import org.apache.ignite.lang.IgnitePredicate; +import org.apache.ignite.lang.IgniteRunnable; +import org.apache.ignite.spi.systemview.view.SystemView; +import org.apache.ignite.spi.systemview.view.CacheGroupView; +import org.apache.ignite.spi.systemview.view.CacheView; +import org.apache.ignite.spi.systemview.view.ComputeTaskView; +import org.apache.ignite.spi.systemview.view.ServiceView; +import org.apache.ignite.internal.processors.service.DummyService; +import org.apache.ignite.internal.util.typedef.F; +import org.apache.ignite.services.ServiceConfiguration; +import org.apache.ignite.testframework.GridTestUtils; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.Test; + +import static org.apache.ignite.internal.processors.cache.ClusterCachesInfo.CACHES_VIEW; +import static org.apache.ignite.internal.processors.cache.ClusterCachesInfo.CACHE_GRPS_VIEW; +import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SVCS_VIEW; +import static org.apache.ignite.internal.processors.task.GridTaskProcessor.TASKS_VIEW; + +/** Tests for {@link SystemView}. */ +public class SystemViewSelfTest extends GridCommonAbstractTest { +/** Tests work of {@link SystemView} for caches. */ +@Test +public void testCachesView() throws Exception { +try (IgniteEx g = startGrid()) { +Set cacheNames = new HashSet<>(Arrays.asList("cache-1", "cache-2")); + +for (String name : cacheNames) +g.createCache(name); + +SystemView caches = g.context().systemView().view(CACHES_VIEW); + +assertEquals(g.context().cache().cacheDescriptors().size(), F.size(caches.iterator())); + +for (CacheView row : caches) +cacheNames.remove(row.cacheName()); + +assertTrue(cacheNames.toString(), cacheNames.isEmpty()); +} +} + +/** Tests work of {@link SystemView} for cache groups. */ +@Test +public void testCacheGroupsView() throws Exception { +try (IgniteEx g = startGrid()) { +Set grpNames = new HashSet<>(Arrays.asList("grp-1", "grp-2")); + +for (String grpName : grpNames) +g.createCache(new CacheConfiguration<>("cache-" + grpName).setGroupName(grpName)); + +SystemView grps = g.context().systemView().view(CACHE_GRPS_VIEW); + +assertEquals(g.context().cache().cacheGroupDescriptors().size(), F.size(grps.iterator())); + +for (CacheGroupView row : grps) +grpNames.remove(row.cacheGroupName()); + +assertTrue(grpNames.toString(), grpNames.isEmpty()); +} +} + +/** Tests work of {@link SystemView} for services. */ +@Test +public void testServices() throws Exception { +try (IgniteEx g = startGrid()) { +{ +ServiceConfiguration srvcCfg =
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r325256658 ## File path: modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcMetadataSelfTest.java ## @@ -320,10 +319,8 @@ public void testGetTableTypes() throws Exception { */ @Test public void testGetAllView() throws Exception { -List expViews = Arrays.asList( +Set expViews = new HashSet<>(Arrays.asList( "BASELINE_NODES", -"CACHES", -"CACHE_GROUPS", Review comment: I see your point. Let's discuss it on the dev-list? Seems, it's up to community consensus. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r324737194 ## File path: modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java ## @@ -0,0 +1,144 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.metric; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.internal.IgniteEx; +import org.apache.ignite.spi.metric.view.SystemView; +import org.apache.ignite.spi.metric.view.CacheGroupView; +import org.apache.ignite.spi.metric.view.CacheView; +import org.apache.ignite.spi.metric.view.ComputeTaskView; +import org.apache.ignite.spi.metric.view.ServiceView; +import org.apache.ignite.internal.processors.service.DummyService; +import org.apache.ignite.internal.util.typedef.F; +import org.apache.ignite.services.ServiceConfiguration; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; + +import static org.apache.ignite.internal.processors.cache.ClusterCachesInfo.CACHES_VIEW; +import static org.apache.ignite.internal.processors.cache.ClusterCachesInfo.CACHE_GRPS_VIEW; +import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SVCS_VIEW; +import static org.apache.ignite.internal.processors.task.GridTaskProcessor.TASKS_VIEW; + +/** */ +public class SystemViewSelfTest extends GridCommonAbstractTest { +/** */ +private static CountDownLatch latch; + +/** */ +@Test +public void testCachesView() throws Exception { +try (IgniteEx g = startGrid()) { +Set cacheNames = new HashSet<>(Arrays.asList("cache-1", "cache-2")); + +for (String name : cacheNames) +g.createCache(name); + +SystemView caches = g.context().metric().view(CACHES_VIEW); + +assertEquals(g.context().cache().cacheDescriptors().size(), F.size(caches.iterator())); + +for (CacheView row : caches) +cacheNames.remove(row.cacheName()); + +assertTrue(cacheNames.toString(), cacheNames.isEmpty()); +} +} + +/** */ +@Test +public void testCacheGroupsView() throws Exception { +try(IgniteEx g = startGrid()) { +Set grpNames = new HashSet<>(Arrays.asList("grp-1", "grp-2")); + +for (String grpName : grpNames) +g.createCache(new CacheConfiguration<>("cache-" + grpName).setGroupName(grpName)); + +SystemView grps = g.context().metric().view(CACHE_GRPS_VIEW); + +assertEquals(g.context().cache().cacheGroupDescriptors().size(), F.size(grps.iterator())); + +for (CacheGroupView row : grps) +grpNames.remove(row.cacheGroupName()); + +assertTrue(grpNames.toString(), grpNames.isEmpty()); +} +} + +/** */ +@Test +public void testServices() throws Exception { +try(IgniteEx g = startGrid()) { +ServiceConfiguration srvcCfg = new ServiceConfiguration(); + +srvcCfg.setName("service"); +srvcCfg.setMaxPerNodeCount(1); +srvcCfg.setService(new DummyService()); + +g.services().deploy(srvcCfg); + +SystemView srvs = g.context().metric().view(SVCS_VIEW); + +assertEquals(g.context().service().serviceDescriptors().size(), F.size(srvs.iterator())); + +ServiceView sview = srvs.iterator().next(); + +assertEquals(srvcCfg.getName(), sview.name()); +assertEquals(srvcCfg.getMaxPerNodeCount(), sview.maxPerNodeCount()); +assertEquals(DummyService.class, sview.serviceClass()); +} +} + +/** */ +@Test +public void testComputeBroadcast() throws Exception { Review comment: Fixed!!! 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r324141819 ## File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlSystemViewsSelfTest.java ## @@ -1293,7 +1302,7 @@ public void testCachesViews() throws Exception { assertEquals(0L, execSql("SELECT COUNT(*) FROM " + systemSchemaName() + ".CACHES WHERE CACHE_NAME = 'no_such_cache'").get(0) .get(0)); -assertEquals(0L, execSql("SELECT COUNT(*) FROM " + systemSchemaName() + ".CACHES WHERE CACHE_NAME = 1").get(0).get(0)); +//assertEquals(0L, execSql("SELECT COUNT(*) FROM " + systemSchemaName() + ".CACHES WHERE CACHE_NAME = 1").get(0).get(0)); Review comment: Actually, this is error in the existing test. `CACHE_NAME` is a string and 1 is number. Fixed, for now. Thanks! 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323730801 ## File path: modules/core/src/main/java/org/apache/ignite/spi/metric/list/MonitoringRowAttributeWalker.java ## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.spi.metric.list; + +/** + * Utility class for quick iteration over {@link MonitoringRow} properties. + */ +public interface MonitoringRowAttributeWalker { +/** @return Count of a row properties. */ +public int count(); + +/** + * Calls visitor for each {@link MonitoringRow} attribute. + * + * @param visitor Attribute visitor. + */ +public void visitAll(AttributeVisitor visitor); + +/** + * Calls visitor for each {@link MonitoringRow} attribute. + * Value of the attribute also provided. + * + * @param row Row to iterate. + * @param visitor Attribute visitor. + */ +public void visitAll(R row, AttributeWithValueVisitor visitor); + +/** Attribute visitor. */ +public interface AttributeVisitor { +/** + * Visit some object property. + * @param idx Index. + * @param name Name. + * @param clazz Value class. + * @param Value type. + */ +public void accept(int idx, String name, Class clazz); + +/** + * Visit attribute. Attribute value is {@code boolean} primitive. + * + * @param idx Index. + * @param name Name. + */ +public void acceptBoolean(int idx, String name); Review comment: Seems, we simplify and make code more quick with the accept* for primitives. Isn't it? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323728128 ## File path: modules/core/src/main/java/org/apache/ignite/spi/metric/list/MonitoringRow.java ## @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.spi.metric.list; + +import org.apache.ignite.internal.processors.metric.GridMetricManager; +import org.apache.ignite.spi.metric.ReadOnlyMonitoringListRegistry; + +/** + * Monitoring list row base interface. + * Each row idenitified by the instance of {@code Id}. Review comment: It mean "obsolete" comment :). Removed 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323689797 ## File path: modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java ## @@ -7040,6 +7042,51 @@ public static String toShortString(Collection ns) { return sb.a(']').toString(); } +/** + * Get string representation of an object properly catching all exceptions. + * + * @param obj Object. + * @return Result or {@code null}. + */ +@Nullable public static String toStringSafe(@Nullable Object obj) { +if (obj == null) +return null; +else { +try { +return obj.toString(); +} +catch (Exception e) { +try { +return "Failed to convert object to string: " + e.getMessage(); +} +catch (Exception e0) { +return "Failed to convert object to string (error message is not available)"; +} +} +} +} + +/** + * @param objs Collection of objects. + * @return String representation. + */ +@Nullable public static String toStringSafe(@Nullable Collection objs) { Review comment: `CacheGroupView#affinity` `CacheView#topologyValidator` Collection case will be use in the next PRs, removed. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323689797 ## File path: modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java ## @@ -7040,6 +7042,51 @@ public static String toShortString(Collection ns) { return sb.a(']').toString(); } +/** + * Get string representation of an object properly catching all exceptions. + * + * @param obj Object. + * @return Result or {@code null}. + */ +@Nullable public static String toStringSafe(@Nullable Object obj) { +if (obj == null) +return null; +else { +try { +return obj.toString(); +} +catch (Exception e) { +try { +return "Failed to convert object to string: " + e.getMessage(); +} +catch (Exception e0) { +return "Failed to convert object to string (error message is not available)"; +} +} +} +} + +/** + * @param objs Collection of objects. + * @return String representation. + */ +@Nullable public static String toStringSafe(@Nullable Collection objs) { Review comment: `CacheGroupView#affinity` `CacheView#topologyValidator` 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323678802 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/GridMetricManager.java ## @@ -265,47 +379,127 @@ 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 iterator() { -return registries.values().iterator(); +/** + * Registers list which exports {@link ConcurrentMap} content. + * + * @param name Name of the list. + * @param desc Description of the list. + * @param rowCls Row class. + * @param data Data of the list. + * @param rowFunc value to row function. + * @param rowClearer Function that clears data on list removal. + * @param List row type. + * @param Map data type. + */ +public void registerList(String name, String desc, +Class rowCls, ConcurrentMap data, Function rowFunc, Consumer rowClearer) { + +Supplier> listCreator = () -> (MonitoringList)lists.computeIfAbsent(name, n -> { +MonitoringList list = new MonitoringListAdapter<>(name, +desc, +rowCls, +(MonitoringRowAttributeWalker)walkers.get(rowCls), +data, +rowFunc); + +notifyListeners(list, listCreationLsnrs, log); + +return list; +}); + +// Create new instance of the list. +listCreator.get(); + +ctx.metric().addEnableListListener(name, n -> listCreator.get()); Review comment: Fixed. > Methods addEnableListListener and addRemoveListListener can be removed. But these methods still required. Can you clarify, how it can be removed? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323678265 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/GridMetricManager.java ## @@ -177,9 +222,63 @@ /** Registered metrics registries. */ private final ConcurrentHashMap registries = new ConcurrentHashMap<>(); +/** Metrics registry. */ +private final ReadOnlyMetricRegistry metricsRegistry = new ReadOnlyMetricRegistry() { +/** {@inheritDoc} */ +@NotNull @Override public Iterator iterator() { +return registries.values().iterator(); +} + +/** {@inheritDoc} */ +@Override public void addMetricRegistryCreationListener(Consumer lsnr) { +metricRegCreationLsnrs.add(lsnr); +} + +/** {@inheritDoc} */ +@Override public void addMetricRegistryRemoveListener(Consumer lsnr) { +metricRegRemoveLsnrs.add(lsnr); +} +}; + +/** Registered lists. */ +private final ConcurrentHashMap> lists = new ConcurrentHashMap<>(); + +/** Lists registry. */ +private final ReadOnlyMonitoringListRegistry listRegistry = new ReadOnlyMonitoringListRegistry() { +/** {@inheritDoc} */ +@Override public void addListCreationListener(Consumer> lsnr) { +listCreationLsnrs.add(lsnr); +} + +/** {@inheritDoc} */ +@Override public void addListRemoveListener(Consumer> lsnr) { +listRemoveLsnrs.add(lsnr); +} + +/** {@inheritDoc} */ +@NotNull @Override public Iterator> iterator() { +return lists.values().iterator(); +} +}; + /** Metric registry creation listeners. */ private final List> metricRegCreationLsnrs = new CopyOnWriteArrayList<>(); +/** Metric registry remove listeners. */ +private final List> metricRegRemoveLsnrs = new CopyOnWriteArrayList<>(); + +/** List creation listeners. */ +private final List>> listCreationLsnrs = new CopyOnWriteArrayList<>(); + +/** List remove listeners. */ +private final List>> listRemoveLsnrs = new CopyOnWriteArrayList<>(); + +/** Internal list remove listeners. */ +private final ConcurrentMap>> internalRemoveLsnrs = new ConcurrentHashMap<>(); + +/** List enable listeners. */ +private final ConcurrentMap> listEnableLsnrs = new ConcurrentHashMap<>(); Review comment: Renamed to `task`. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323629603 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/list/MonitoringListAdapter.java ## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.metric.list; + +import java.util.Iterator; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.jetbrains.annotations.NotNull; + +/** + * Monitoring list backed by {@code data} {@link ConcurrentMap}. + */ +public class MonitoringListAdapter extends AbstractMonitoringList { +/** Data backed by this list. */ +private final ConcurrentMap data; Review comment: We don't need a `Collection` backed implementation of monitoring list for now. If we need it in the future, we will rework this code. In the previous comments, you propose do not create code, we don't need right now. For now, all we need is an implementation that backed some `ConcurrentMap`. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323628460 ## 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 iterator() { -return registries.values().iterator(); +public void list(String name, String description, +Class rowClazz, Supplier> data, Function rowFunc, Consumer clearer) { + +Supplier> listCreator = () -> list(name, () -> new MonitoringListAdapter<>(name, +description, +rowClazz, +(MonitoringRowAttributeWalker)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: OK, fixed. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323595410 ## 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 iterator() { -return registries.values().iterator(); +public void list(String name, String description, +Class rowClazz, Supplier> data, Function rowFunc, Consumer clearer) { Review comment: OK, fixed. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323593975 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/list/MonitoringListAdapter.java ## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.metric.list; + +import java.util.Iterator; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.jetbrains.annotations.NotNull; + +/** + * Monitoring list backed by {@code data} {@link ConcurrentMap}. + */ +public class MonitoringListAdapter extends AbstractMonitoringList { +/** Data backed by this list. */ +private final ConcurrentMap data; Review comment: > And Map say nothing about thread safety. Thats why I use `ConcurrentMap` which declares: ``` * A {@link java.util.Map} providing thread safety and atomicity * guarantees. ``` If we use `Collection` we can't force "user" of the list to pass thread-safety implementation of Collection. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323592931 ## 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 iterator() { -return registries.values().iterator(); +public void list(String name, String description, +Class rowClazz, Supplier> data, Function rowFunc, Consumer clearer) { + +Supplier> listCreator = () -> list(name, () -> new MonitoringListAdapter<>(name, +description, +rowClazz, +(MonitoringRowAttributeWalker)walkers.get(rowClazz), +data.get(), +rowFunc)); + +//Create new instance of the list. +listCreator.get(); + +ctx.metric().addEnableListListener(listenOnlyEqual(name, identity(), n -> listCreator.get())); +ctx.metric().addRemoveListListener(l -> data.get().values().forEach(clearer)); } -/** {@inheritDoc} */ -@Override public void addMetricRegistryCreationListener(Consumer lsnr) { -metricRegCreationLsnrs.add(lsnr); +/** + * @param name Name of the list. + * @return List. + */ +@Nullable public MonitoringList list(String name) { +return (MonitoringList)lists.get(name); +} + +/** + * Gets or creates {@link MonitoringList}. + * + * @param name Name of the list. + * @param listSupplier List supplier. + * @param Type of the row. + * @return Monitoring list. + */ +private MonitoringList list(String name, Review comment: OK, fixed. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323591739 ## File path: modules/codegen/src/main/java/org/apache/ignite/codegen/MonitoringRowAttributeWalkerGenerator.java ## @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.codegen; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.ObjIntConsumer; +import org.apache.ignite.internal.processors.metric.list.walker.Order; +import org.apache.ignite.spi.metric.jmx.MonitoringListMBean; +import org.apache.ignite.spi.metric.list.MonitoringList; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.apache.ignite.spi.metric.list.view.CacheGroupView; +import org.apache.ignite.spi.metric.list.view.CacheView; +import org.apache.ignite.spi.metric.list.view.ComputeTaskView; +import org.apache.ignite.spi.metric.list.view.ServiceView; +import org.apache.ignite.spi.metric.sql.MonitoringListLocalSystemView; + +import static org.apache.ignite.codegen.MessageCodeGenerator.DFLT_SRC_DIR; Review comment: It's not a path for `MonitoringRowAttributeWalkerGenerator`, it's a path of source in the core module. You should use another constant if you want to generate code in another module. But if the path for the core source changes, I think we want to change only one constant. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323591133 ## 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 iterator() { -return registries.values().iterator(); +public void list(String name, String description, +Class rowClazz, Supplier> data, Function rowFunc, Consumer clearer) { + +Supplier> listCreator = () -> list(name, () -> new MonitoringListAdapter<>(name, +description, +rowClazz, +(MonitoringRowAttributeWalker)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: > Create maps for list creators and removers (name -> closure) instead of lists for listeners Yes, we can do it. But: 1. This will require one more additional listener map because we already have 'List' of the listeners that comes from `MonitoringExporterSpi`. Please, see `ReadOnlyMonitoringListRegistry#addListRemoveListener`. 2. I expect very few enable/disable events, and from 10 to 20 lists over Ignite, so we shouldn't observe performance issues because of name check in `listenOnlyEqual`. So I prefer to keep it as is. What do you think? > Get rid of as most nested lambdas as possible Seems, I'm already did it. Can you see, how lambdas can be reduced? What is the issue with the lambdas count, in the first place? Do you see some issues with code readability, understandability? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323448695 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/list/MonitoringListAdapter.java ## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.metric.list; + +import java.util.Iterator; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.jetbrains.annotations.NotNull; + +/** + * Monitoring list backed by {@code data} {@link ConcurrentMap}. + */ +public class MonitoringListAdapter extends AbstractMonitoringList { +/** Data backed by this list. */ +private final ConcurrentMap data; Review comment: Because Map is not a Collection, I think :) And simple Map doesn't provide thread safe iterator. We can use another implementation of `MonitoringList` if we need it. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323450872 ## 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 iterator() { -return registries.values().iterator(); +public void list(String name, String description, +Class rowClazz, Supplier> data, Function rowFunc, Consumer clearer) { + +Supplier> listCreator = () -> list(name, () -> new MonitoringListAdapter<>(name, +description, +rowClazz, +(MonitoringRowAttributeWalker)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 doesn't understand your proposal, actually. What is the different between "enable list event" and "list enabler"? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323448695 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/list/MonitoringListAdapter.java ## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.metric.list; + +import java.util.Iterator; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.jetbrains.annotations.NotNull; + +/** + * Monitoring list backed by {@code data} {@link ConcurrentMap}. + */ +public class MonitoringListAdapter extends AbstractMonitoringList { +/** Data backed by this list. */ +private final ConcurrentMap data; Review comment: Because Map is not a Collection, I think :) And simple Map doesn't provide thread safe iterator. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323448695 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/list/MonitoringListAdapter.java ## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.metric.list; + +import java.util.Iterator; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.jetbrains.annotations.NotNull; + +/** + * Monitoring list backed by {@code data} {@link ConcurrentMap}. + */ +public class MonitoringListAdapter extends AbstractMonitoringList { +/** Data backed by this list. */ +private final ConcurrentMap data; Review comment: Because Map is not a Collection, I think :) 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323447996 ## 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 iterator() { -return registries.values().iterator(); +public void list(String name, String description, +Class rowClazz, Supplier> data, Function rowFunc, Consumer clearer) { Review comment: A map may be recreated in the class who owns data. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323294796 ## 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 iterator() { -return registries.values().iterator(); +public void list(String name, String description, +Class rowClazz, Supplier> data, Function rowFunc, Consumer clearer) { + +Supplier> listCreator = () -> list(name, () -> new MonitoringListAdapter<>(name, +description, +rowClazz, +(MonitoringRowAttributeWalker)walkers.get(rowClazz), +data.get(), +rowFunc)); + +//Create new instance of the list. +listCreator.get(); + +ctx.metric().addEnableListListener(listenOnlyEqual(name, identity(), n -> listCreator.get())); +ctx.metric().addRemoveListListener(l -> data.get().values().forEach(clearer)); } -/** {@inheritDoc} */ -@Override public void addMetricRegistryCreationListener(Consumer lsnr) { -metricRegCreationLsnrs.add(lsnr); +/** + * @param name Name of the list. + * @return List. + */ +@Nullable public MonitoringList list(String name) { +return (MonitoringList)lists.get(name); +} + +/** + * Gets or creates {@link MonitoringList}. + * + * @param name Name of the list. + * @param listSupplier List supplier. + * @param Type of the row. + * @return Monitoring list. + */ +private MonitoringList list(String name, Review comment: It will be more usages in the follow-up PRs. Let's keep it? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323287034 ## 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 iterator() { -return registries.values().iterator(); +public void list(String name, String description, +Class rowClazz, Supplier> data, Function rowFunc, Consumer clearer) { + +Supplier> listCreator = () -> list(name, () -> new MonitoringListAdapter<>(name, +description, +rowClazz, +(MonitoringRowAttributeWalker)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: 1. We create a list which exports map content. 2. On list removal event(via `IgniteMBean#disableList`) `rowClearer` will clear data needed only for list(if applicable). 3. On list enable event(via `IgniteMBean#enableList`) it will be recreated(fresh instance of the `data` map will be obtained. I think it pretty simple and easy to use approach. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323270197 ## File path: modules/codegen/src/main/java/org/apache/ignite/codegen/MonitoringRowAttributeWalkerGenerator.java ## @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.codegen; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.ObjIntConsumer; +import org.apache.ignite.internal.processors.metric.list.walker.Order; +import org.apache.ignite.spi.metric.jmx.MonitoringListMBean; +import org.apache.ignite.spi.metric.list.MonitoringList; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.apache.ignite.spi.metric.list.view.CacheGroupView; +import org.apache.ignite.spi.metric.list.view.CacheView; +import org.apache.ignite.spi.metric.list.view.ComputeTaskView; +import org.apache.ignite.spi.metric.list.view.ServiceView; +import org.apache.ignite.spi.metric.sql.MonitoringListLocalSystemView; + +import static org.apache.ignite.codegen.MessageCodeGenerator.DFLT_SRC_DIR; Review comment: Why it should be independent? Constants created to be reused, isn't it? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323270197 ## File path: modules/codegen/src/main/java/org/apache/ignite/codegen/MonitoringRowAttributeWalkerGenerator.java ## @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.codegen; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.ObjIntConsumer; +import org.apache.ignite.internal.processors.metric.list.walker.Order; +import org.apache.ignite.spi.metric.jmx.MonitoringListMBean; +import org.apache.ignite.spi.metric.list.MonitoringList; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.apache.ignite.spi.metric.list.view.CacheGroupView; +import org.apache.ignite.spi.metric.list.view.CacheView; +import org.apache.ignite.spi.metric.list.view.ComputeTaskView; +import org.apache.ignite.spi.metric.list.view.ServiceView; +import org.apache.ignite.spi.metric.sql.MonitoringListLocalSystemView; + +import static org.apache.ignite.codegen.MessageCodeGenerator.DFLT_SRC_DIR; Review comment: Why it should be independent? Constants created to be resused, istn't it? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323270197 ## File path: modules/codegen/src/main/java/org/apache/ignite/codegen/MonitoringRowAttributeWalkerGenerator.java ## @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.codegen; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.ObjIntConsumer; +import org.apache.ignite.internal.processors.metric.list.walker.Order; +import org.apache.ignite.spi.metric.jmx.MonitoringListMBean; +import org.apache.ignite.spi.metric.list.MonitoringList; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.apache.ignite.spi.metric.list.view.CacheGroupView; +import org.apache.ignite.spi.metric.list.view.CacheView; +import org.apache.ignite.spi.metric.list.view.ComputeTaskView; +import org.apache.ignite.spi.metric.list.view.ServiceView; +import org.apache.ignite.spi.metric.sql.MonitoringListLocalSystemView; + +import static org.apache.ignite.codegen.MessageCodeGenerator.DFLT_SRC_DIR; Review comment: Why it should be independent? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r323268245 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/list/AbstractMonitoringList.java ## @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.metric.list; + +import org.apache.ignite.spi.metric.list.MonitoringList; +import org.apache.ignite.spi.metric.list.MonitoringRow; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; + +/** + * + */ +abstract class AbstractMonitoringList implements MonitoringList { +/** Name of the list. */ +private final String name; + +/** Description of the list. */ +private final String description; + +/** Class of the row */ +private final Class rowClass; + +/** + * Row attribute walker. + * + * @see org.apache.ignite.codegen.MonitoringRowAttributeWalkerGenerator Review comment: So it's the issue in the IDEA, isn't it?) 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r322466293 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java ## @@ -959,29 +959,29 @@ public void initQueryStructuresForNotStartedCache(DynamicCacheDescriptor cacheDe */ @SuppressWarnings({"unchecked"}) private void stopCache(GridCacheAdapter cache, boolean cancel, boolean destroy) { -GridCacheContext ctx = cache.context(); +GridCacheContext cctx = cache.context(); Review comment: Reverted. Thanks. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r322464862 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/list/walker/CacheGroupViewWalker.java ## @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.metric.list.walker; + +import org.apache.ignite.cache.CacheAtomicityMode; +import org.apache.ignite.cache.CacheMode; +import org.apache.ignite.cache.CacheRebalanceMode; +import org.apache.ignite.cache.PartitionLossPolicy; +import org.apache.ignite.spi.metric.list.MonitoringRowAttributeWalker; +import org.apache.ignite.spi.metric.list.view.CacheGroupView; + +/** + * Generated by {@code org.apache.ignite.codegen.MonitoringRowAttributeWalkerGenerator}. + * {@link CacheGroupView} attributes walker. + * + * @see CacheGroupView + */ +public class CacheGroupViewWalker implements MonitoringRowAttributeWalker { +/** {@inheritDoc} */ +@Override public void visitAll(AttributeVisitor v) { +v.accept(0, "cacheGroupName", String.class); Review comment: Actually, this code was generated by `MonitoringRowAttributeWalkerGenerator`. You can find its source code in this PR. So, when you change some view class, you simply run it and got up to date walker implementation. What do you think? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r322464245 ## File path: modules/core/src/main/java/org/apache/ignite/spi/metric/list/MonitoringRow.java ## @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.spi.metric.list; + +import org.apache.ignite.internal.processors.metric.GridMetricManager; +import org.apache.ignite.spi.metric.ReadOnlyMonitoringListRegistry; + +/** + * Monitoring list row base interface. + * Each row idenitified by the instance of {@code Id}. + * + * @see MonitoringList + * @see GridMetricManager + * @see ReadOnlyMonitoringListRegistry + */ +public interface MonitoringRow { Review comment: Hmm, it's a good point. Thanks! I think the answer is no. We are not going to provide general serializability. We can track some `Thread`s or other not serializable data later. What do you think? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r322462720 ## File path: modules/core/src/main/java/org/apache/ignite/spi/metric/list/MonitoringList.java ## @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.spi.metric.list; + +/** + * Implementation provides data for some internal Ignite objects. + * + * @param Type of the row identificator. + * @param Type of the row. + */ +public interface MonitoringList> extends Iterable { Review comment: @alex-plekhanov @agura Guys, what do you think about naming? 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r322441256 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ClusterCachesInfo.java ## @@ -145,6 +173,14 @@ public ClusterCachesInfo(GridKernalContext ctx) { this.ctx = ctx; +ctx.metric().list(CACHES_MON_LIST, CACHES_MON_LIST_DESC, CacheView.class, +l -> cachesMonList = l, +l -> cachesMonList = null); + +ctx.metric().list(CACHE_GRPS_MON_LIST, CACHE_GRPS_MON_LIST_DESC, CacheGroupView.class, +l -> cachesGrpMonList = l, +l -> cachesGrpMonList = null); + Review comment: All lists made consistent after enable/disable. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r322312751 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ClusterCachesInfo.java ## @@ -2514,7 +2572,7 @@ public void removeRestartingCaches() { if (o1.cacheType().userCache() ^ o2.cacheType().userCache()) return o2.cacheType().userCache() ? -1 : 1; -return o1.cacheId().compareTo(o2.cacheId()); +return o1.cacheId() - o2.cacheId(); Review comment: It's better to use primitives in `MonitoringRow` for the sake of performance. Replaced with `Integer.compare(o1.cacheId(), o2.cacheId())` 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r322314665 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ClusterCachesInfo.java ## @@ -145,6 +173,14 @@ public ClusterCachesInfo(GridKernalContext ctx) { this.ctx = ctx; +ctx.metric().list(CACHES_MON_LIST, CACHES_MON_LIST_DESC, CacheView.class, +l -> cachesMonList = l, +l -> cachesMonList = null); + +ctx.metric().list(CACHE_GRPS_MON_LIST, CACHE_GRPS_MON_LIST_DESC, CacheGroupView.class, +l -> cachesGrpMonList = l, +l -> cachesGrpMonList = null); + Review comment: It'is expected behavior, from my point of view. It's not easy to build a consistent list if it enabled in runtime. Transactions list, locks list, can be taken as an example. 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
[GitHub] [ignite] nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine.
nizhikov commented on a change in pull request #6845: IGNITE-12145: Monitoring list engine. URL: https://github.com/apache/ignite/pull/6845#discussion_r322312751 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ClusterCachesInfo.java ## @@ -2514,7 +2572,7 @@ public void removeRestartingCaches() { if (o1.cacheType().userCache() ^ o2.cacheType().userCache()) return o2.cacheType().userCache() ? -1 : 1; -return o1.cacheId().compareTo(o2.cacheId()); +return o1.cacheId() - o2.cacheId(); Review comment: It's better to use primitives in `MonitoringRow` for the sake of speed. Replaced with `Integer.compare(o1.cacheId(), o2.cacheId())` 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