Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[server] Fix stats handling for SINGLE_GET requests #1381

Merged
merged 3 commits into from
Dec 10, 2024
Merged

[server] Fix stats handling for SINGLE_GET requests #1381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d6b2a9
[server] Fix stats handling for SINGLE_GET requests
nisargthakkar Dec 10, 2024
b98e153
Add tests
nisargthakkar Dec 10, 2024
770c009
Address review comments
nisargthakkar Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions services/venice-server/src/main/java/com/linkedin/venice/stats/ServerHttpRequestStats.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,23 +295,33 @@ public ServerHttpRequestStats(
() -> totalStats.earlyTerminatedEarlyRequestCountSensor,
new OccurrenceRate());

if (isKeyValueProfilingEnabled) {
// size profiling is expensive
if (isKeyValueProfilingEnabled || requestType == RequestType.SINGLE_GET) {
sushantmane marked this conversation as resolved.
Show resolved Hide resolved
final MeasurableStat[] valueSizeStats;
final MeasurableStat[] keySizeStats;
String requestValueSizeSensorName = "request_value_size";
String requestKeySizeSensorName = "request_key_size";
if (isKeyValueProfilingEnabled) {
valueSizeStats = TehutiUtils
.getFineGrainedPercentileStatWithAvgAndMax(getName(), getFullMetricName(requestValueSizeSensorName));
keySizeStats = TehutiUtils
.getFineGrainedPercentileStatWithAvgAndMax(getName(), getFullMetricName(requestKeySizeSensorName));
} else {
valueSizeStats = new MeasurableStat[] { new Avg(), new Max() };
keySizeStats = new MeasurableStat[] { new Avg(), new Max() };
}

requestValueSizeSensor = registerPerStoreAndTotal(
requestValueSizeSensorName,
totalStats,
() -> totalStats.requestValueSizeSensor,
TehutiUtils
.getFineGrainedPercentileStatWithAvgAndMax(getName(), getFullMetricName(requestValueSizeSensorName)));
String requestKeySizeSensorName = "request_key_size";
valueSizeStats);
requestKeySizeSensor = registerPerStoreAndTotal(
requestKeySizeSensorName,
totalStats,
() -> totalStats.requestKeySizeSensor,
TehutiUtils
.getFineGrainedPercentileStatWithAvgAndMax(getName(), getFullMetricName(requestKeySizeSensorName)));
keySizeStats);
}

misroutedStoreVersionSensor = registerPerStoreAndTotal(
"misrouted_store_version_request_count",
totalStats,
Expand Down
67 changes: 56 additions & 11 deletions .../venice-server/src/test/java/com/linkedin/venice/stats/AggServerHttpRequestStatsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@


public class AggServerHttpRequestStatsTest {
protected MetricsRepository metricsRepository;
protected MockTehutiReporter reporter;
protected AggServerHttpRequestStats singleGetStats;
protected AggServerHttpRequestStats batchGetStats;
private MetricsRepository metricsRepository;
private MockTehutiReporter reporter;
private MetricsRepository metricsRepositoryForKVProfiling;
private MockTehutiReporter reporterForKVProfiling;
private AggServerHttpRequestStats singleGetStats;
private AggServerHttpRequestStats singleGetStatsWithKVProfiling;
private AggServerHttpRequestStats batchGetStats;

private static final String STORE_FOO = "store_foo";
private static final String STORE_BAR = "store_bar";
Expand All @@ -26,14 +29,21 @@ public class AggServerHttpRequestStatsTest {
@BeforeTest
public void setUp() {
this.metricsRepository = new MetricsRepository();
Assert.assertEquals(metricsRepository.metrics().size(), 0);
this.reporter = new MockTehutiReporter();
this.metricsRepository.addReporter(reporter);

this.metricsRepositoryForKVProfiling = new MetricsRepository();
this.reporterForKVProfiling = new MockTehutiReporter();
this.metricsRepositoryForKVProfiling.addReporter(reporterForKVProfiling);

Assert.assertEquals(metricsRepository.metrics().size(), 0);
Assert.assertEquals(metricsRepositoryForKVProfiling.metrics().size(), 0);

this.singleGetStats = new AggServerHttpRequestStats(
"test_cluster",
metricsRepository,
RequestType.SINGLE_GET,
true,
false,
Mockito.mock(ReadOnlyStoreRepository.class),
true,
false);
Expand All @@ -45,18 +55,31 @@ public void setUp() {
Mockito.mock(ReadOnlyStoreRepository.class),
true,
false);

this.singleGetStatsWithKVProfiling = new AggServerHttpRequestStats(
"test_cluster",
metricsRepositoryForKVProfiling,
RequestType.SINGLE_GET,
true,
Mockito.mock(ReadOnlyStoreRepository.class),
true,
false);
}

@AfterTest
public void cleanUp() {
metricsRepository.close();
metricsRepositoryForKVProfiling.close();
reporter.close();
reporterForKVProfiling.close();
}

@Test
public void testMetrics() {
ServerHttpRequestStats singleGetServerStatsFoo = singleGetStats.getStoreStats(STORE_FOO);
ServerHttpRequestStats singleGetServerStatsBar = singleGetStats.getStoreStats(STORE_BAR);
ServerHttpRequestStats singleGetServerStatsWithKvProfilingFoo =
singleGetStatsWithKVProfiling.getStoreStats(STORE_FOO);

singleGetServerStatsFoo.recordSuccessRequest();
singleGetServerStatsFoo.recordSuccessRequest();
Expand All @@ -66,6 +89,9 @@ public void testMetrics() {
singleGetServerStatsFoo.recordKeySizeInByte(100);
singleGetServerStatsFoo.recordValueSizeInByte(1000);

singleGetServerStatsWithKvProfilingFoo.recordKeySizeInByte(100);
singleGetServerStatsWithKvProfilingFoo.recordValueSizeInByte(1000);

Assert.assertTrue(
reporter.query("." + STORE_FOO + "--success_request.OccurrenceRate").value() > 0,
"success_request rate should be positive");
Expand All @@ -75,15 +101,34 @@ public void testMetrics() {
Assert.assertTrue(
reporter.query(".total--success_request_ratio.RatioStat").value() > 0,
"success_request_ratio should be positive");
Assert.assertTrue(
reporter.query(".store_foo--request_key_size.Avg").value() > 0,
"Avg value for request key size should always be recorded");
Assert.assertTrue(
reporter.query(".store_foo--request_key_size.Max").value() > 0,
"Max value for request key size should always be recorded");

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" };
Assert.assertTrue(
reporterForKVProfiling.query(".store_foo--request_key_size.Avg").value() > 0,
"Avg value for request key size should always be recorded");
Assert.assertTrue(
reporterForKVProfiling.query(".store_foo--request_key_size.Max").value() > 0,
"Max value for request key size should always be recorded");

String[] fineGrainedPercentiles = 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 (String fineGrainedPercentile: fineGrainedPercentiles) {
Assert.assertNull(
metricsRepository.getMetric(".store_foo--request_key_size." + fineGrainedPercentile + "thPercentile"));
Assert.assertNull(
metricsRepository.getMetric(".store_foo--request_value_size." + fineGrainedPercentile + "thPercentile"));

for (int i = 0; i < percentileStrings.length; i++) {
Assert.assertTrue(
reporter.query(".store_foo--request_key_size." + percentileStrings[i] + "thPercentile").value() > 0);
reporterForKVProfiling.query(".store_foo--request_key_size." + fineGrainedPercentile + "thPercentile")
.value() > 0);
Assert.assertTrue(
reporter.query(".store_foo--request_value_size." + percentileStrings[i] + "thPercentile").value() > 0);
reporterForKVProfiling.query(".store_foo--request_value_size." + fineGrainedPercentile + "thPercentile")
.value() > 0);
}

singleGetStats.handleStoreDeleted(STORE_FOO);
Expand Down
Loading