[router][server] Disable key value profiling for single gets

linkedin · Dec 10, 2024 · 047f2e6 · 047f2e6
1 parent 35bcbef
commit 047f2e6
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 5 deletions.
diff --git a/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/TehutiUtils.java b/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/TehutiUtils.java
@@ -23,7 +23,7 @@ public class TehutiUtils {
   // metrics. It's likely to degrade critical path performance
   private static final double[] FINE_GRAINED_HISTOGRAM_PERCENTILES =
       new double[] { 0.01, 0.1, 1, 2, 3, 4, 5, 10, 20, 30, 40, 50, 60, 70, 80, 90, 95, 99, 99.9 };
-  private static final double[] HISTOGRAM_PERCENTILES_FOR_NETWORK_LATENCY = new double[] { 50, 77, 90, 95, 99, 99.9 };
+  private static final double[] HISTOGRAM_PERCENTILES_FOR_NETWORK_LATENCY = new double[] { 50, 95, 99, 99.9 };
   private static final String ROUND_NUMBER_SUFFIX = ".0";
 
   /**

diff --git a/services/venice-server/src/main/java/com/linkedin/venice/stats/ServerHttpRequestStats.java b/services/venice-server/src/main/java/com/linkedin/venice/stats/ServerHttpRequestStats.java
@@ -295,8 +295,8 @@ public ServerHttpRequestStats(
         () -> totalStats.earlyTerminatedEarlyRequestCountSensor,
         new OccurrenceRate());
 
-    if (isKeyValueProfilingEnabled || requestType == RequestType.SINGLE_GET) {
-      // size profiling is only expensive for requests with lots of keys, but we keep it always on for single gets...
+    if (isKeyValueProfilingEnabled) {
+      // size profiling is expensive
       String requestValueSizeSensorName = "request_value_size";
       requestValueSizeSensor = registerPerStoreAndTotal(
           requestValueSizeSensorName,

diff --git a/.../venice-server/src/test/java/com/linkedin/venice/stats/AggServerHttpRequestStatsTest.java b/.../venice-server/src/test/java/com/linkedin/venice/stats/AggServerHttpRequestStatsTest.java
@@ -33,7 +33,7 @@ public void setUp() {
         "test_cluster",
         metricsRepository,
         RequestType.SINGLE_GET,
-        false,
+        true,
         Mockito.mock(ReadOnlyStoreRepository.class),
         true,
         false);
@@ -63,6 +63,9 @@ public void testMetrics() {
     singleGetServerStatsFoo.recordErrorRequest();
     singleGetServerStatsBar.recordErrorRequest();
 
+    singleGetServerStatsFoo.recordKeySizeInByte(100);
+    singleGetServerStatsFoo.recordValueSizeInByte(1000);
+
     Assert.assertTrue(
         reporter.query("." + STORE_FOO + "--success_request.OccurrenceRate").value() > 0,
         "success_request rate should be positive");
@@ -73,6 +76,16 @@ public void testMetrics() {
         reporter.query(".total--success_request_ratio.RatioStat").value() > 0,
         "success_request_ratio should be positive");
 
+    String[] percentileStrings = new String[] { "0_01", "0_01", "0_1", "1", "2", "3", "4", "5", "10", "20", "30", "40",
+        "50", "60", "70", "80", "90", "95", "99", "99_9" };
+
+    for (int i = 0; i < percentileStrings.length; i++) {
+      Assert.assertTrue(
+          reporter.query(".store_foo--request_key_size." + percentileStrings[i] + "thPercentile").value() > 0);
+      Assert.assertTrue(
+          reporter.query(".store_foo--request_value_size." + percentileStrings[i] + "thPercentile").value() > 0);
+    }
+
     singleGetStats.handleStoreDeleted(STORE_FOO);
     Assert.assertNull(metricsRepository.getMetric("." + STORE_FOO + "--success_request.OccurrenceRate"));
   }
@@ -83,7 +96,7 @@ public void testPercentileNamePattern() {
     String storeName = "storeName";
     Percentiles percentiles = TehutiUtils.getPercentileStatForNetworkLatency(sensorName, storeName);
     percentiles.stats().stream().map(namedMeasurable -> namedMeasurable.name()).forEach(System.out::println);
-    String[] percentileStrings = new String[] { "50", "77", "90", "95", "99", "99_9" };
+    String[] percentileStrings = new String[] { "50", "95", "99", "99_9" };
 
     for (int i = 0; i < percentileStrings.length; i++) {
       String expectedName = sensorName + "--" + storeName + "." + percentileStrings[i] + "thPercentile";