[GitHub] [ignite] timoninmaxim commented on a change in pull request #9768: IGNITE-15650 : Statistics for platform services. V2 : Static version.

[GitBox <gi...@apache.org>]

timoninmaxim commented on a change in pull request #9768:
URL: https://github.com/apache/ignite/pull/9768#discussion_r814834787



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/IgniteServiceProcessor.java
##########
@@ -638,7 +642,7 @@ private void ensure(boolean cond, String desc, @Nullable Object v) {
      */
     private PreparedConfigurations<IgniteUuid> prepareServiceConfigurations(Collection<ServiceConfiguration> cfgs,
         IgnitePredicate<ClusterNode> dfltNodeFilter) {
-        List<ServiceConfiguration> cfgsCp = new ArrayList<>(cfgs.size());
+        List<LazyServiceConfiguration> cfgsCp = new ArrayList<>(cfgs.size());

Review comment:
       In most cases there is no need to change `ServiceConfiguration` to `LazyServiceConfiguration`. The latter inherits from the former, so we can safely use `ServiceConfiguration` everywhere and cast it to `Lazy...` in single place only.

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java
##########
@@ -822,5 +826,7 @@ private static void writeFailedConfiguration(BinaryRawWriterEx w, ServiceConfigu
         if (svcCfg.getNodeFilter() instanceof PlatformClusterNodeFilterImpl)
             dotnetFilter = ((PlatformClusterNodeFilterImpl)svcCfg.getNodeFilter()).getInternalPredicate();
         w.writeObjectDetached(dotnetFilter);
+
+        w.writeBoolean(svcCfg.isStatisticsEnabled());

Review comment:
       Should we write known methods here?

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core.Tests/Services/ServicesAsyncWrapper.cs
##########
@@ -170,6 +170,7 @@ public ICollection<IServiceDescriptor> GetServiceDescriptors()
         }
 
         /** <inheritDoc /> */
+#pragma warning disable 618

Review comment:
       I'm not sure, but this `pragma` is the single change in the file, do we actually need it?

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/LazyServiceConfiguration.java
##########
@@ -40,6 +40,10 @@
     /** */
     private byte[] srvcBytes;
 
+    /** Known service methods to build service statistics. */
+    @GridToStringExclude
+    private String[] knownMtdNames;

Review comment:
       It used only for platform services and then this field is very misleading. I propose to store those methods inside of `PlatformService`. Is it possible?




-- 
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: notifications-unsubscribe@ignite.apache.org

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