This is an automated email from the ASF dual-hosted git repository.
snlee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 1ee4aac5f7 deprecate _gaugeValues used in AbstractMetrics (#10182)
1ee4aac5f7 is described below
commit 1ee4aac5f75b9b71397095330bb9631fbe65745d
Author: Haitao Zhang <[email protected]>
AuthorDate: Fri Jan 27 11:52:40 2023 -0800
deprecate _gaugeValues used in AbstractMetrics (#10182)
* deprecate _gaugeValues used in AbstractMetrics
* address comments
---
.../pinot/common/metrics/AbstractMetrics.java | 37 +++++++---------------
.../MergeRollupMinionClusterIntegrationTest.java | 26 +++++++--------
2 files changed, 24 insertions(+), 39 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
b/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
index f1044abd77..778568fcf0 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
@@ -18,7 +18,6 @@
*/
package org.apache.pinot.common.metrics;
-import com.google.common.annotations.VisibleForTesting;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
@@ -44,8 +43,6 @@ import org.slf4j.LoggerFactory;
/**
* Common code for metrics implementations.
- * TODO: 1. With gauge updatable, we can remove _gaugeValues 2. Remove methods
with callback in name since the callback
- * function can not be updated.
*/
public abstract class AbstractMetrics<QP extends AbstractMetrics.QueryPhase, M
extends AbstractMetrics.Meter,
G extends AbstractMetrics.Gauge, T extends AbstractMetrics.Timer> {
@@ -58,6 +55,9 @@ public abstract class AbstractMetrics<QP extends
AbstractMetrics.QueryPhase, M e
private final Class _clazz;
+ // The purpose of having _gaugeValues is to make gauge metric updatable.
+ // Since gauge metric itself is updatable now
(https://github.com/apache/pinot/pull/9961), we can deprecate it.
+ @Deprecated
private final Map<String, AtomicLong> _gaugeValues = new
ConcurrentHashMap<String, AtomicLong>();
private final boolean _isTableLevelMetricsEnabled;
@@ -326,12 +326,16 @@ public abstract class AbstractMetrics<QP extends
AbstractMetrics.QueryPhase, M e
}
/**
+ * @deprecated Please use addMeteredTableValue(final String tableName, final
M meter, final long unitCount), which is
+ * designed for tracking count and rates.
+ *
* Logs a value to a table gauge.
*
* @param tableName The table name
* @param gauge The gauge to use
* @param unitCount The number of units to add to the gauge
*/
+ @Deprecated
public void addValueToTableGauge(final String tableName, final G gauge,
final long unitCount) {
final String fullGaugeName = composeTableGaugeName(tableName, gauge);
@@ -425,11 +429,15 @@ public abstract class AbstractMetrics<QP extends
AbstractMetrics.QueryPhase, M e
}
/**
+ * @deprecated Please use addMeteredGlobalValue(final M meter, final long
unitCount), which is designed for tracking
+ * count and rates.
+ *
* Adds a value to a table gauge.
*
* @param gauge The gauge to use
* @param unitCount The number of units to add to the gauge
*/
+ @Deprecated
public void addValueToGlobalGauge(final G gauge, final long unitCount) {
String gaugeName = gauge.getGaugeName();
@@ -448,19 +456,6 @@ public abstract class AbstractMetrics<QP extends
AbstractMetrics.QueryPhase, M e
}
}
- /**
- * Gets the value of a table gauge.
- *
- * @param tableName The table name
- * @param gauge The gauge to use
- */
- public long getValueOfTableGauge(final String tableName, final G gauge) {
- final String fullGaugeName = composeTableGaugeName(tableName, gauge);
-
- AtomicLong gaugeValue = _gaugeValues.get(fullGaugeName);
- return gaugeValue == null ? 0 : gaugeValue.get();
- }
-
/**
* Initializes all global meters (such as exceptions count) to zero.
*/
@@ -732,14 +727,4 @@ public abstract class AbstractMetrics<QP extends
AbstractMetrics.QueryPhase, M e
protected String getTableName(String tableName) {
return _isTableLevelMetricsEnabled || _allowedTables.contains(tableName) ?
tableName : "allTables";
}
-
- /**
- * Check if the metric name appears in the gauge value map.
- * @param metricName metric name
- * @return True if the metric name appears on the gauge value map. False
otherwise.
- */
- @VisibleForTesting
- public boolean containsGauge(String metricName) {
- return _gaugeValues.containsKey(metricName);
- }
}
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MergeRollupMinionClusterIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MergeRollupMinionClusterIntegrationTest.java
index 3c6b98c6e2..9012a21fa0 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MergeRollupMinionClusterIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MergeRollupMinionClusterIntegrationTest.java
@@ -33,6 +33,7 @@ import org.apache.commons.io.FileUtils;
import org.apache.helix.task.TaskState;
import org.apache.pinot.common.lineage.SegmentLineageAccessHelper;
import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.common.metrics.MetricValueUtils;
import org.apache.pinot.common.minion.MergeRollupTaskMetadata;
import org.apache.pinot.common.minion.MinionTaskMetadataUtils;
import org.apache.pinot.common.utils.SqlResultComparator;
@@ -393,9 +394,8 @@ public class MergeRollupMinionClusterIntegrationTest
extends BaseClusterIntegrat
// Check total tasks
assertEquals(numTasks, 5);
- assertTrue(_controllerStarter.getControllerMetrics()
-
.containsGauge("mergeRollupTaskDelayInNumBuckets.myTable1_OFFLINE.100days"));
-
+
assertTrue(MetricValueUtils.gaugeExists(_controllerStarter.getControllerMetrics(),
+ "mergeRollupTaskDelayInNumBuckets.myTable1_OFFLINE.100days"));
// Drop the table
dropOfflineTable(SINGLE_LEVEL_CONCAT_TEST_TABLE);
@@ -506,8 +506,8 @@ public class MergeRollupMinionClusterIntegrationTest
extends BaseClusterIntegrat
// Check total tasks
assertEquals(numTasks, 5);
- assertTrue(_controllerStarter.getControllerMetrics()
-
.containsGauge("mergeRollupTaskDelayInNumBuckets.myTable4_OFFLINE.100days"));
+
assertTrue(MetricValueUtils.gaugeExists(_controllerStarter.getControllerMetrics(),
+ "mergeRollupTaskDelayInNumBuckets.myTable4_OFFLINE.100days"));
// Drop the table
dropOfflineTable(SINGLE_LEVEL_CONCAT_METADATA_TEST_TABLE);
@@ -624,8 +624,8 @@ public class MergeRollupMinionClusterIntegrationTest
extends BaseClusterIntegrat
// Check total tasks
assertEquals(numTasks, 3);
- assertTrue(_controllerStarter.getControllerMetrics()
-
.containsGauge("mergeRollupTaskDelayInNumBuckets.myTable2_OFFLINE.150days"));
+
assertTrue(MetricValueUtils.gaugeExists(_controllerStarter.getControllerMetrics(),
+ "mergeRollupTaskDelayInNumBuckets.myTable2_OFFLINE.150days"));
}
/**
@@ -765,10 +765,10 @@ public class MergeRollupMinionClusterIntegrationTest
extends BaseClusterIntegrat
// Check total tasks
assertEquals(numTasks, 8);
- assertTrue(_controllerStarter.getControllerMetrics()
-
.containsGauge("mergeRollupTaskDelayInNumBuckets.myTable3_OFFLINE.45days"));
- assertTrue(_controllerStarter.getControllerMetrics()
-
.containsGauge("mergeRollupTaskDelayInNumBuckets.myTable3_OFFLINE.90days"));
+
assertTrue(MetricValueUtils.gaugeExists(_controllerStarter.getControllerMetrics(),
+ "mergeRollupTaskDelayInNumBuckets.myTable3_OFFLINE.45days"));
+
assertTrue(MetricValueUtils.gaugeExists(_controllerStarter.getControllerMetrics(),
+ "mergeRollupTaskDelayInNumBuckets.myTable3_OFFLINE.90days"));
}
protected void verifyTableDelete(String tableNameWithType) {
@@ -892,8 +892,8 @@ public class MergeRollupMinionClusterIntegrationTest
extends BaseClusterIntegrat
// Check total tasks
assertEquals(numTasks, 5);
- assertTrue(_controllerStarter.getControllerMetrics()
-
.containsGauge("mergeRollupTaskDelayInNumBuckets.myTable5_REALTIME.100days"));
+
assertTrue(MetricValueUtils.gaugeExists(_controllerStarter.getControllerMetrics(),
+ "mergeRollupTaskDelayInNumBuckets.myTable5_REALTIME.100days"));
// Drop the table
dropRealtimeTable(tableName);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
