You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by jl...@apache.org on 2019/10/29 01:56:12 UTC

[incubator-pinot] 01/01: Clean up duplicate/unused metrics in controller

This is an automated email from the ASF dual-hosted git repository.

jlli pushed a commit to branch clean-up-metrics
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git

commit 09931aa6dd46149effe2716d64dfc9bc86d1176a
Author: jackjlli <jl...@linkedin.com>
AuthorDate: Mon Oct 28 18:55:50 2019 -0700

    Clean up duplicate/unused metrics in controller
---
 .../pinot/common/metrics/ControllerMeter.java      |  4 ---
 .../pinot/common/metrics/ValidationMetrics.java    | 37 ----------------------
 .../realtime/PinotLLCRealtimeSegmentManager.java   |  2 --
 .../core/realtime/SegmentCompletionManager.java    |  5 ---
 4 files changed, 48 deletions(-)

diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java
index 65a05f9..39ea8db 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java
@@ -37,14 +37,10 @@ public enum ControllerMeter implements AbstractMetrics.Meter {
   CONTROLLER_TABLE_TENANT_CREATE_ERROR("TableTenantCreateError", true),
   CONTROLLER_TABLE_TENANT_DELETE_ERROR("TableTenantDeleteError", true),
   CONTROLLER_REALTIME_TABLE_SEGMENT_ASSIGNMENT_ERROR("errors", true),
-  CONTROLLER_NOT_LEADER("notLeader", true),
   CONTROLLER_LEADERSHIP_CHANGE_WITHOUT_CALLBACK("leadershipChangeWithoutCallback", true),
   LLC_STATE_MACHINE_ABORTS("aborts", false),
   LLC_ZOOKEEPER_FETCH_FAILURES("failures", false),
   LLC_ZOOKEEPER_UPDATE_FAILURES("failures", false),
-  LLC_KAFKA_DATA_LOSS("dataLoss", false),
-  // Introducing a new stream agnostic metric to replace LLC_KAFKA_DATA_LOSS.
-  // We can phase out LLC_KAFKA_DATA_LOSS once we have collected sufficient metrics for the new one
   LLC_STREAM_DATA_LOSS("dataLoss", false),
   CONTROLLER_PERIODIC_TASK_RUN("periodicTaskRun", false),
   CONTROLLER_PERIODIC_TASK_ERROR("periodicTaskError", false),
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ValidationMetrics.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ValidationMetrics.java
index 8f4b4fd..9b87b74 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ValidationMetrics.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ValidationMetrics.java
@@ -54,29 +54,6 @@ public class ValidationMetrics {
   }
 
   /**
-   * A simple gauge that returns the difference between the current system time in millis and the value stored in the
-   * _gaugeValues hash map.
-   */
-  private class CurrentTimeMillisDeltaGauge extends Gauge<Long> {
-    private final String key;
-
-    public CurrentTimeMillisDeltaGauge(String key) {
-      this.key = key;
-    }
-
-    @Override
-    public Long value() {
-      Long gaugeValue = _gaugeValues.get(key);
-
-      if (gaugeValue != null && gaugeValue != Long.MIN_VALUE) {
-        return System.currentTimeMillis() - gaugeValue;
-      } else {
-        return Long.MIN_VALUE;
-      }
-    }
-  }
-
-  /**
    * A simple gauge that returns the difference in hours between the current system time and the value stored in the
    * _gaugeValues hash map.
    */
@@ -112,13 +89,6 @@ public class ValidationMetrics {
     }
   }
 
-  private class CurrentTimeMillisDeltaGaugeFactory implements GaugeFactory<Long> {
-    @Override
-    public Gauge<Long> buildGauge(final String key) {
-      return new CurrentTimeMillisDeltaGauge(key);
-    }
-  }
-
   private class CurrentTimeMillisDeltaGaugeHoursFactory implements GaugeFactory<Double> {
     @Override
     public Gauge<Double> buildGauge(final String key) {
@@ -127,8 +97,6 @@ public class ValidationMetrics {
   }
 
   private final StoredValueGaugeFactory _storedValueGaugeFactory = new StoredValueGaugeFactory();
-  private final CurrentTimeMillisDeltaGaugeFactory _currentTimeMillisDeltaGaugeFactory =
-      new CurrentTimeMillisDeltaGaugeFactory();
   private final CurrentTimeMillisDeltaGaugeHoursFactory _currentTimeMillisDeltaGaugeHoursFactory =
       new CurrentTimeMillisDeltaGaugeHoursFactory();
 
@@ -160,9 +128,6 @@ public class ValidationMetrics {
    *                               if there is no such time.
    */
   public void updateOfflineSegmentDelayGauge(final String resource, final long lastOfflineSegmentTime) {
-    final String fullGaugeName = makeGaugeName(resource, "offlineSegmentDelayMillis");
-    makeGauge(fullGaugeName, makeMetricName(fullGaugeName), _currentTimeMillisDeltaGaugeFactory,
-        lastOfflineSegmentTime);
     final String fullGaugeNameHours = makeGaugeName(resource, "offlineSegmentDelayHours");
     makeGauge(fullGaugeNameHours, makeMetricName(fullGaugeNameHours), _currentTimeMillisDeltaGaugeHoursFactory,
         lastOfflineSegmentTime);
@@ -176,8 +141,6 @@ public class ValidationMetrics {
    *                           such time.
    */
   public void updateLastPushTimeGauge(final String resource, final long lastPushTimeMillis) {
-    final String fullGaugeName = makeGaugeName(resource, "lastPushTimeDelayMillis");
-    makeGauge(fullGaugeName, makeMetricName(fullGaugeName), _currentTimeMillisDeltaGaugeFactory, lastPushTimeMillis);
     final String fullGaugeNameHours = makeGaugeName(resource, "lastPushTimeDelayHours");
     makeGauge(fullGaugeNameHours, makeMetricName(fullGaugeNameHours), _currentTimeMillisDeltaGaugeHoursFactory,
         lastPushTimeMillis);
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
index ef4fe68..1a654ed 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
@@ -891,7 +891,6 @@ public class PinotLLCRealtimeSegmentManager {
             if (partitionStartOffset > startOffset) {
               LOGGER.error("Data lost from offset: {} to: {} for partition: {} of table: {}", startOffset,
                   partitionStartOffset, partitionId, realtimeTableName);
-              _controllerMetrics.addMeteredTableValue(realtimeTableName, ControllerMeter.LLC_KAFKA_DATA_LOSS, 1L);
               _controllerMetrics.addMeteredTableValue(realtimeTableName, ControllerMeter.LLC_STREAM_DATA_LOSS, 1L);
               startOffset = partitionStartOffset;
             }
@@ -938,7 +937,6 @@ public class PinotLLCRealtimeSegmentManager {
             LOGGER
                 .error("Failed to find previous CONSUMING segment for partition: {} of table: {}, potential data loss",
                     partitionId, realtimeTableName);
-            _controllerMetrics.addMeteredTableValue(realtimeTableName, ControllerMeter.LLC_KAFKA_DATA_LOSS, 1L);
             _controllerMetrics.addMeteredTableValue(realtimeTableName, ControllerMeter.LLC_STREAM_DATA_LOSS, 1L);
           }
           updateInstanceStatesForNewConsumingSegment(instanceStatesMap, previousConsumingSegment, latestSegmentName,
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/SegmentCompletionManager.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/SegmentCompletionManager.java
index d76837e..b467495 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/SegmentCompletionManager.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/SegmentCompletionManager.java
@@ -165,7 +165,6 @@ public class SegmentCompletionManager {
     final String segmentNameStr = reqParams.getSegmentName();
     final String tableName = segmentNameStr.split(SEPARATOR)[0];
     if (!isLeader(tableName) || !_helixManager.isConnected()) {
-      _controllerMetrics.addMeteredGlobalValue(ControllerMeter.CONTROLLER_NOT_LEADER, 1L);
       return SegmentCompletionProtocol.RESP_NOT_LEADER;
     }
     final String instanceId = reqParams.getInstanceId();
@@ -204,7 +203,6 @@ public class SegmentCompletionManager {
     final String segmentNameStr = reqParams.getSegmentName();
     final String tableName = segmentNameStr.split(SEPARATOR)[0];
     if (!isLeader(tableName) || !_helixManager.isConnected()) {
-      _controllerMetrics.addMeteredGlobalValue(ControllerMeter.CONTROLLER_NOT_LEADER, 1L);
       return SegmentCompletionProtocol.RESP_NOT_LEADER;
     }
     final String instanceId = reqParams.getInstanceId();
@@ -229,7 +227,6 @@ public class SegmentCompletionManager {
     final String segmentNameStr = reqParams.getSegmentName();
     final String tableName = segmentNameStr.split(SEPARATOR)[0];
     if (!isLeader(tableName) || !_helixManager.isConnected()) {
-      _controllerMetrics.addMeteredGlobalValue(ControllerMeter.CONTROLLER_NOT_LEADER, 1L);
       return SegmentCompletionProtocol.RESP_NOT_LEADER;
     }
     final String instanceId = reqParams.getInstanceId();
@@ -261,7 +258,6 @@ public class SegmentCompletionManager {
     final String segmentNameStr = reqParams.getSegmentName();
     final String tableName = segmentNameStr.split(SEPARATOR)[0];
     if (!isLeader(tableName) || !_helixManager.isConnected()) {
-      _controllerMetrics.addMeteredGlobalValue(ControllerMeter.CONTROLLER_NOT_LEADER, 1L);
       return SegmentCompletionProtocol.RESP_NOT_LEADER;
     }
     final String instanceId = reqParams.getInstanceId();
@@ -298,7 +294,6 @@ public class SegmentCompletionManager {
     final String segmentNameStr = reqParams.getSegmentName();
     final String tableName = segmentNameStr.split(SEPARATOR)[0];
     if (!isLeader(tableName) || !_helixManager.isConnected()) {
-      _controllerMetrics.addMeteredGlobalValue(ControllerMeter.CONTROLLER_NOT_LEADER, 1L);
       return SegmentCompletionProtocol.RESP_NOT_LEADER;
     }
     LLCSegmentName segmentName = new LLCSegmentName(segmentNameStr);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org