Skip to content

Commit

Permalink
[router][server] Reduce the granularity of latency histogram percenti…
Browse files Browse the repository at this point in the history
…les and disable key value profiling for single gets (#1377)

Currently, we record fine-grained percentiles for key and value of all single get requests. This bloats up the metrics significantly. This commit restricts key and value profiling only for cases where this config is enabled.

In addition to this, removed P77 and P90 metrics from latency metrics. We still have p50, p95, p99 and p99.9.
  • Loading branch information
nisargthakkar authored Dec 10, 2024
1 parent 6c7e13f commit 6111853
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void setUp() {
"test_cluster",
metricsRepository,
RequestType.SINGLE_GET,
false,
true,
Mockito.mock(ReadOnlyStoreRepository.class),
true,
false);
Expand Down Expand Up @@ -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");
Expand All @@ -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"));
}
Expand All @@ -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";
Expand Down

0 comments on commit 6111853

Please sign in to comment.