Skip to content

Commit dd4a58e

Browse files
authored
[server] Create RecordLevelDelayOtelStats for per record timestamp tracking (linkedin#2567)
1 parent 90a4d30 commit dd4a58e

2 files changed

Lines changed: 57 additions & 12 deletions

File tree

clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/ingestion/heartbeat/HeartbeatVersionedStats.java

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -245,36 +245,49 @@ private RecordLevelDelayOtelStats getOrCreateRecordLevelDelayOtelStats(String st
245245

246246
/**
247247
* Emits a per-record OTel metric for leader record delay (called per record, not aggregated).
248-
* Uses {@code get()} instead of {@code getOrCreate} to avoid synchronization on this hot path.
249-
* Returns early if stats haven't been initialized yet for this store.
248+
* Uses {@code get()} as a fast path; falls back to {@code getOrCreate} on first call per store.
250249
*/
251250
public void emitPerRecordLeaderOtelMetric(String storeName, int version, String region, long delay) {
252-
RecordLevelDelayOtelStats otelStats = recordLevelDelayOtelStatsMap.get(storeName);
251+
RecordLevelDelayOtelStats otelStats = getOrLazilyCreateRecordLevelDelayOtelStats(storeName);
253252
if (otelStats == null || !otelStats.emitOtelMetrics()) {
254-
return; // Fast path exit: stats not initialized or OTel disabled
253+
return;
255254
}
256255
otelStats.recordRecordDelayOtelMetrics(version, region, ReplicaType.LEADER, ReplicaState.READY_TO_SERVE, delay);
257256
}
258257

259258
/**
260259
* Emits a per-record OTel metric for follower record delay (called per record, not aggregated).
261-
* Uses {@code get()} instead of {@code getOrCreate} to avoid synchronization on this hot path.
262-
* Returns early if stats haven't been initialized yet for this store.
260+
* Uses {@code get()} as a fast path; falls back to {@code getOrCreate} on first call per store.
263261
*/
264262
public void emitPerRecordFollowerOtelMetric(
265263
String storeName,
266264
int version,
267265
String region,
268266
long delay,
269267
boolean isReadyToServe) {
270-
RecordLevelDelayOtelStats otelStats = recordLevelDelayOtelStatsMap.get(storeName);
268+
RecordLevelDelayOtelStats otelStats = getOrLazilyCreateRecordLevelDelayOtelStats(storeName);
271269
if (otelStats == null || !otelStats.emitOtelMetrics()) {
272-
return; // Fast path exit: stats not initialized or OTel disabled
270+
return;
273271
}
274272
ReplicaState replicaState = isReadyToServe ? ReplicaState.READY_TO_SERVE : ReplicaState.CATCHING_UP;
275273
otelStats.recordRecordDelayOtelMetrics(version, region, ReplicaType.FOLLOWER, replicaState, delay);
276274
}
277275

276+
/**
277+
* Fast-path lookup with lazy initialization fallback.
278+
* Returns null if the store is not found in the metadata repository (e.g., store was deleted).
279+
*/
280+
private RecordLevelDelayOtelStats getOrLazilyCreateRecordLevelDelayOtelStats(String storeName) {
281+
RecordLevelDelayOtelStats existing = recordLevelDelayOtelStatsMap.get(storeName);
282+
if (existing != null) {
283+
return existing;
284+
}
285+
if (!metadataRepository.hasStore(storeName)) {
286+
return null;
287+
}
288+
return getOrCreateRecordLevelDelayOtelStats(storeName);
289+
}
290+
278291
@VisibleForTesting
279292
HeartbeatStat getStatsForTesting(String storeName, int version) {
280293
return getStats(storeName, version);

clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/ingestion/heartbeat/HeartbeatVersionedStatsTest.java

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ public void setUp() {
9393
when(mockStore.getVersions()).thenReturn(versions);
9494

9595
when(mockMetadataRepository.getStoreOrThrow(STORE_NAME)).thenReturn(mockStore);
96+
when(mockMetadataRepository.hasStore(STORE_NAME)).thenReturn(true);
9697
when(mockMetadataRepository.getAllStores()).thenReturn(Collections.singletonList(mockStore));
9798

9899
leaderMonitors = new VeniceConcurrentHashMap<>();
@@ -495,12 +496,43 @@ public void testEmitPerRecordFollowerOtelMetric(boolean isReadyToServe) {
495496
}
496497

497498
@Test
498-
public void testEmitPerRecordOtelMetricWhenStoreNotInitialized() {
499-
// Test that emitting metrics for an unknown store doesn't throw exception
500-
// This tests the null check fast path - should be a graceful no-op
499+
public void testEmitPerRecordOtelMetricWhenStoreNotInMetadataRepository() {
500+
// Should be a graceful no-op: no exception thrown, no stats entry created
501501
heartbeatVersionedStats.emitPerRecordLeaderOtelMetric("unknown_store", 1, REGION, 100);
502502
heartbeatVersionedStats.emitPerRecordFollowerOtelMetric("unknown_store", 1, REGION, 100, true);
503-
// No exception should be thrown - graceful no-op since recordLevelDelayOtelStatsMap.get() returns null
503+
504+
assertNull(
505+
heartbeatVersionedStats.getRecordLevelDelayOtelStatsForTesting("unknown_store"),
506+
"No stats entry should be created for a store not in the metadata repository");
507+
}
508+
509+
/**
510+
* Verifies that emitPerRecordLeaderOtelMetric works without the periodic path
511+
* (recordLeaderRecordLag) ever being called first
512+
*/
513+
@Test
514+
public void testEmitPerRecordLeaderOtelMetricWithoutPeriodicInitialization() {
515+
heartbeatVersionedStats.setCurrentTimeSupplier(() -> FIXED_CURRENT_TIME);
516+
517+
// Call emitPerRecordLeaderOtelMetric directly without any prior recordLeaderRecordLag call
518+
heartbeatVersionedStats.emitPerRecordLeaderOtelMetric(STORE_NAME, CURRENT_VERSION, REGION, 100);
519+
heartbeatVersionedStats.emitPerRecordLeaderOtelMetric(STORE_NAME, CURRENT_VERSION, REGION, 200);
520+
heartbeatVersionedStats.emitPerRecordLeaderOtelMetric(STORE_NAME, CURRENT_VERSION, REGION, 150);
521+
522+
validateRecordOtelHistogram(ReplicaType.LEADER, ReplicaState.READY_TO_SERVE, 100.0, 200.0, 3, 450.0);
523+
}
524+
525+
@Test(dataProvider = "True-and-False", dataProviderClass = DataProviderUtils.class)
526+
public void testEmitPerRecordFollowerOtelMetricWithoutPeriodicInitialization(boolean isReadyToServe) {
527+
heartbeatVersionedStats.setCurrentTimeSupplier(() -> FIXED_CURRENT_TIME);
528+
529+
// Call emitPerRecordFollowerOtelMetric directly without any prior recordFollowerRecordLag call
530+
heartbeatVersionedStats.emitPerRecordFollowerOtelMetric(STORE_NAME, CURRENT_VERSION, REGION, 100, isReadyToServe);
531+
heartbeatVersionedStats.emitPerRecordFollowerOtelMetric(STORE_NAME, CURRENT_VERSION, REGION, 200, isReadyToServe);
532+
heartbeatVersionedStats.emitPerRecordFollowerOtelMetric(STORE_NAME, CURRENT_VERSION, REGION, 150, isReadyToServe);
533+
534+
ReplicaState activeState = isReadyToServe ? ReplicaState.READY_TO_SERVE : ReplicaState.CATCHING_UP;
535+
validateRecordOtelHistogram(ReplicaType.FOLLOWER, activeState, 100.0, 200.0, 3, 450.0);
504536
}
505537

506538
// ==================================================================================

0 commit comments

Comments
 (0)