address review comments

Author: m-nagarajan

clients/da-vinci-client/src/test/java/com/linkedin/davinci/kafka/consumer/ActiveKeyCountScenarioTest.java

@@ -348,8 +348,10 @@ public void testInvalidationParityRecordsToBothTehutiAndOtel() {
             .build());
     AsyncGauge.AsyncGaugeExecutor asyncGaugeExecutor = new AsyncGauge.AsyncGaugeExecutor.Builder().build();
     TestMockTime mockTime = new TestMockTime();
-    // 3-arg ctor wires mockTime into MetricsRepository.measure(now); the 1-arg MetricConfig form
-    // silently uses SystemTime, which would defeat the time-advance below.
+    /*
+     * 3-arg ctor wires mockTime into MetricsRepository.measure(now); the 1-arg MetricConfig form
+     * silently uses SystemTime, which would defeat the time-advance below.
+     */
     MetricsRepository tehutiRepo =
         new MetricsRepository(new MetricConfig(asyncGaugeExecutor), Collections.emptyList(), mockTime);
 
@@ -386,8 +388,10 @@ public void testInvalidationParityRecordsToBothTehutiAndOtel() {
           ACTIVE_KEY_COUNT_INVALIDATION.getMetricEntity().getMetricName(),
           TEST_PREFIX);
 
-      // LongAdderRateGauge caches its value for RATE_GAUGE_CACHE_DURATION_IN_SECONDS; advance past
-      // the window to force a fresh measurement.
+      /*
+       * LongAdderRateGauge caches its value for RATE_GAUGE_CACHE_DURATION_IN_SECONDS; advance past
+       * the window to force a fresh measurement.
+       */
       mockTime.addMilliseconds(LongAdderRateGauge.RATE_GAUGE_CACHE_DURATION_IN_SECONDS * Time.MS_PER_SECOND);
       assertEquals(
           tehutiRepo.getMetric(".total--active_key_count_invalidation.Rate").value(),

clients/da-vinci-client/src/test/java/com/linkedin/davinci/kafka/consumer/ActiveKeyCountTest.java

@@ -476,7 +476,7 @@ public void testBatchPutReturnsValueOperationType(int schemaId, String desc) {
   }
 
   @Test(dataProvider = "putRmdWithSchemaIds")
-  public void testPutInStorageEngine_batchRmdEnabled_writesRmd(int schemaId, String desc) throws Exception {
+  public void testPutInStorageEngineBatchRmdEnabledWritesRmd(int schemaId, String desc) throws Exception {
     setupForStorageEngineTests(true);
     ingestionTask.putInStorageEngine(PARTITION, KEY_BYTES, createBatchPut(schemaId));
     verify(storageEngine)
@@ -485,7 +485,7 @@ public void testPutInStorageEngine_batchRmdEnabled_writesRmd(int schemaId, Strin
   }
 
   @Test
-  public void testPutInStorageEngine_batchRmdEnabled_chunkFragment_fallsThrough() throws Exception {
+  public void testPutInStorageEngineBatchRmdEnabledChunkFragmentFallsThrough() throws Exception {
     setupForStorageEngineTests(true);
     ingestionTask.putInStorageEngine(PARTITION, KEY_BYTES, createBatchPut(CHUNK_SCHEMA_ID));
     verify(storageEngine).put(anyInt(), any(byte[].class), any(ByteBuffer.class));
@@ -494,8 +494,7 @@ public void testPutInStorageEngine_batchRmdEnabled_chunkFragment_fallsThrough()
   }
 
   @Test(dataProvider = "putFallThroughCases")
-  public void testPutInStorageEngine_fallsThrough(boolean addRmdEnabled, boolean postEop, String desc)
-      throws Exception {
+  public void testPutInStorageEngineFallsThrough(boolean addRmdEnabled, boolean postEop, String desc) throws Exception {
     setupForStorageEngineTests(addRmdEnabled);
     doReturn(postEop).when(pcs).isEndOfPushReceived();
     ingestionTask.putInStorageEngine(PARTITION, KEY_BYTES, createBatchPut(USER_SCHEMA_ID));
@@ -505,7 +504,7 @@ public void testPutInStorageEngine_fallsThrough(boolean addRmdEnabled, boolean p
   }
 
   @Test
-  public void testPutInStorageEngine_batchRmdEnabled_pcsNull_fallsThrough() throws Exception {
+  public void testPutInStorageEngineBatchRmdEnabledPcsNullFallsThrough() throws Exception {
     setupPcsNullFallThrough();
     doCallRealMethod().when(ingestionTask).putInStorageEngine(anyInt(), any(), any(Put.class));
     doReturn(ActiveActiveStoreIngestionTask.StorageOperationType.VALUE).when(ingestionTask)
@@ -520,15 +519,15 @@ public void testPutInStorageEngine_batchRmdEnabled_pcsNull_fallsThrough() throws
   // regardless of addRmdToBatchPushForHybridStores, so the ts=0 sentinel only applies to PUTs.
 
   @Test
-  public void testRemoveFromStorageEngine_batchDeleteAlwaysUsesPlainDelete() throws Exception {
+  public void testRemoveFromStorageEngineBatchDeleteAlwaysUsesPlainDelete() throws Exception {
     setupForStorageEngineTests(true); // addRmdEnabled=true, but DELETE should still use plain delete
     ingestionTask.removeFromStorageEngine(PARTITION, KEY_BYTES, createBatchDelete());
     verify(storageEngine).delete(anyInt(), any(byte[].class));
     verify(storageEngine, never()).deleteWithReplicationMetadata(anyInt(), any(byte[].class), any(byte[].class));
   }
 
   @Test
-  public void testRemoveFromStorageEngine_postEop_goesToValueAndRmd() throws Exception {
+  public void testRemoveFromStorageEnginePostEopGoesToValueAndRmd() throws Exception {
     setupForStorageEngineTests(true);
     doReturn(true).when(pcs).isEndOfPushReceived();
     ingestionTask.removeFromStorageEngine(PARTITION, KEY_BYTES, createBatchDelete());
@@ -540,7 +539,7 @@ public void testRemoveFromStorageEngine_postEop_goesToValueAndRmd() throws Excep
   // trackActiveKeyCount
 
   @Test
-  public void testTrackActiveKeyCount_batchCounting_schemaFiltering() throws Exception {
+  public void testTrackActiveKeyCountBatchCountingSchemaFiltering() throws Exception {
     setupForTrackActiveKeyCount(true, false, false);
     // Non-chunked PUT and manifest: counted
     for (int schemaId: new int[] { USER_SCHEMA_ID, CHUNK_MANIFEST_SCHEMA_ID }) {
@@ -565,7 +564,7 @@ public void testTrackActiveKeyCount_batchCounting_schemaFiltering() throws Excep
   }
 
   @Test(dataProvider = "batchCountingSkippedCases")
-  public void testTrackActiveKeyCount_batchCounting_skipped(
+  public void testTrackActiveKeyCountBatchCountingSkipped(
       boolean batchEnabled,
       boolean postEop,
       MessageType messageType,
@@ -583,7 +582,7 @@ public void testTrackActiveKeyCount_batchCounting_skipped(
   }
 
   @Test
-  public void testTrackActiveKeyCount_batchAndFollower_bothEnabled() throws Exception {
+  public void testTrackActiveKeyCountBatchAndFollowerBothEnabled() throws Exception {
     setupForTrackActiveKeyCount(true, true, true);
     PartitionConsumptionState mockPcs = createMockPcsForTrack(false, -1L);
     invokeTrackActiveKeyCount(
@@ -597,7 +596,7 @@ public void testTrackActiveKeyCount_batchAndFollower_bothEnabled() throws Except
   }
 
   @Test
-  public void testTrackActiveKeyCount_followerSignal_createdAndDeleted() throws Exception {
+  public void testTrackActiveKeyCountFollowerSignalCreatedAndDeleted() throws Exception {
     setupForTrackActiveKeyCount(false, true, true);
     // Created signal
     PartitionConsumptionState mockPcs1 = createMockPcsForTrack(true, 5L);
@@ -671,7 +670,7 @@ public void testFollowerSignalInvalidatesCount(
   }
 
   @Test(dataProvider = "followerSignalSkippedCases")
-  public void testTrackActiveKeyCount_followerSignal_skipped(
+  public void testTrackActiveKeyCountFollowerSignalSkipped(
       boolean hybridEnabled,
       boolean isAA,
       boolean postEop,
@@ -693,7 +692,7 @@ public void testTrackActiveKeyCount_followerSignal_skipped(
   }
 
   @Test
-  public void testTrackActiveKeyCount_followerSignal_chunkFiltering() throws Exception {
+  public void testTrackActiveKeyCountFollowerSignalChunkFiltering() throws Exception {
     setupForTrackActiveKeyCount(false, true, true);
     // Manifest: signal applied
     PartitionConsumptionState manifestPcs = createMockPcsForTrack(true, 5L);
@@ -716,7 +715,7 @@ public void testTrackActiveKeyCount_followerSignal_chunkFiltering() throws Excep
   }
 
   @Test
-  public void testTrackActiveKeyCount_followerSignal_headerAbsentOrInvalid() throws Exception {
+  public void testTrackActiveKeyCountFollowerSignalHeaderAbsentOrInvalid() throws Exception {
     setupForTrackActiveKeyCount(false, true, true);
     // Test missing header, null value header, and empty byte array header
     PubSubMessageHeaders[] headerCases =
@@ -739,7 +738,7 @@ public void testTrackActiveKeyCount_followerSignal_headerAbsentOrInvalid() throw
   // processMessageAndMaybeProduceToKafka
 
   @Test
-  public void testProcessMessage_signalIncrementAndDecrement() throws Exception {
+  public void testProcessMessageSignalIncrementAndDecrement() throws Exception {
     setupForProcessMessageTests(true);
     doReturn(true).when(pcs).isEndOfPushReceived();
     doReturn(5L).when(pcs).getActiveKeyCount();
@@ -775,7 +774,7 @@ public void testProcessMessage_signalIncrementAndDecrement() throws Exception {
   }
 
   @Test
-  public void testProcessMessage_noCountChange() throws Exception {
+  public void testProcessMessageNoCountChange() throws Exception {
     setupForProcessMessageTests(true);
     doReturn(true).when(pcs).isEndOfPushReceived();
     doReturn(5L).when(pcs).getActiveKeyCount();
@@ -801,7 +800,7 @@ public void testProcessMessage_noCountChange() throws Exception {
   }
 
   @Test
-  public void testProcessMessage_signalSkipped_featureDisabled() throws Exception {
+  public void testProcessMessageSignalSkippedFeatureDisabled() throws Exception {
     setupForProcessMessageTests(false);
     doReturn(true).when(pcs).isEndOfPushReceived();
     doReturn(5L).when(pcs).getActiveKeyCount();
@@ -908,7 +907,7 @@ public void testInvalidateActiveKeyCountLogsEvenWhenInternalsThrow() throws Exce
   }
 
   @Test
-  public void testProcessMessage_midRecordInvalidation_propagatesInvalidateSignal() throws Exception {
+  public void testMidRecordInvalidationPropagatesInvalidateSignal() throws Exception {
     setupForProcessMessageTests(true);
     doReturn(true).when(pcs).isEndOfPushReceived();
     // PCS count is -1 (invalidated mid-record by keyExists failure), but the wrapper
@@ -993,7 +992,7 @@ private void setupForValueLookup(boolean keyExists) throws Exception {
   }
 
   @Test
-  public void testWasOldValueAlive_allBranches() throws Exception {
+  public void testWasOldValueAliveAllBranches() throws Exception {
     // Feature disabled → always false
     setField(ingestionTask, "activeKeyCountForHybridStoreEnabled", false);
     Assert
@@ -1048,7 +1047,7 @@ public void testWasOldValueAlive_allBranches() throws Exception {
   }
 
   @Test
-  public void testIsValuePresentForKey_allTiers() throws Exception {
+  public void testIsValuePresentForKeyAllTiers() throws Exception {
     setField(ingestionTask, "storageEngine", storageEngine);
 
     // Tier 1: Lazy already resolved by DCR → returns cached result
@@ -1077,7 +1076,7 @@ public void testIsValuePresentForKey_allTiers() throws Exception {
   }
 
   @Test
-  public void testIsValuePresentForKey_tier3_chunkedStore_usesChunkingSuffix() throws Exception {
+  public void testIsValuePresentForKeyTier3ChunkedStoreUsesChunkingSuffix() throws Exception {
     setField(ingestionTask, "storageEngine", storageEngine);
     // Enable chunking on the ingestion task
     setField(ingestionTask, "isChunked", true);
@@ -1103,7 +1102,7 @@ public void testIsValuePresentForKey_tier3_chunkedStore_usesChunkingSuffix() thr
   }
 
   @Test
-  public void testIsValuePresentForKey_tier3_nonChunkedStore_usesRawKey() throws Exception {
+  public void testIsValuePresentForKeyTier3NonChunkedStoreUsesRawKey() throws Exception {
     setField(ingestionTask, "storageEngine", storageEngine);
     // Chunking disabled
     setField(ingestionTask, "isChunked", false);
@@ -1119,7 +1118,7 @@ public void testIsValuePresentForKey_tier3_nonChunkedStore_usesRawKey() throws E
   }
 
   @Test
-  public void testIsValuePresentForKey_tier3_keyExistsThrows_invalidatesAndReturnsFalse() throws Exception {
+  public void testIsValuePresentForKeyTier3KeyExistsThrowsInvalidatesAndReturnsFalse() throws Exception {
     setField(ingestionTask, "storageEngine", storageEngine);
     setField(ingestionTask, "isChunked", false);
     doReturn(false).when(ingestionTask).isChunked();

clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/AggHostLevelIngestionStatsTest.java

@@ -290,8 +290,10 @@ public void testActiveKeyCountMetricsAbsentWhenDisabled() {
 
   @Test
   public void testActiveKeyCountMetricsRegisteredWhenEnabled() {
-    // Do NOT close localRepo: closing shuts down the JVM-static DEFAULT_ASYNC_GAUGE_EXECUTOR and
-    // breaks every subsequent AsyncGauge in this JVM.
+    /*
+     * Do NOT close localRepo: closing shuts down the JVM-static DEFAULT_ASYNC_GAUGE_EXECUTOR and
+     * breaks every subsequent AsyncGauge in this JVM.
+     */
     TestMockTime time = new TestMockTime();
     MetricsRepository localRepo = new MetricsRepository(time);
     VeniceServerConfig enabledConfig = mock(VeniceServerConfig.class);