[GitHub] [ignite] ingvard commented on a change in pull request #8145: IGNITE-13349: Tcp discovery statistics migrated to the new metrics framework.

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

ingvard commented on a change in pull request #8145:
URL: https://github.com/apache/ignite/pull/8145#discussion_r471371426



##########
File path: modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoverySpi.java
##########
@@ -2115,6 +2115,20 @@ protected void onExchange(DiscoveryDataPacket dataPacket, ClassLoader clsLdr) {
     @Override public void spiStart(@Nullable String igniteInstanceName) throws IgniteSpiException {
         initializeImpl();
 
+        MetricRegistry discoReg =
+            ((IgniteEx)ignite()).context().metric().registry(metricName("io", "discovery"));

Review comment:
       Do we want to make a set of constants for ("io", "discovery")?

##########
File path: modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/internal/TcpDiscoveryStatistics.java
##########
@@ -287,23 +158,18 @@ public synchronized void onMessageProcessingStarted(TcpDiscoveryAbstractMessage
     public synchronized void onMessageProcessingFinished(TcpDiscoveryAbstractMessage msg) {
         assert msg != null;
 
-        Long startTs = msgsProcStartTs.get(msg.id());
+        Long startTs = msgsProcStartTs.remove(msg.id());
 
-        if (startTs != null) {
+        if (startTs == null) {

Review comment:
       Is it mistake?  long duration = U.currentTimeMillis() - startTs;  startTs is always null.




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