Removed the time property from VenicePath since the only time it is used

by tests to inject a MockTime, it is done from a subclass, and therefore can be achieved differently via extension, rather than composition.
linkedin · Dec 19, 2024 · 17fe9ac · 17fe9ac
1 parent 62102dd
commit 17fe9ac
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 19 deletions.
diff --git a/services/venice-router/src/main/java/com/linkedin/venice/router/api/path/VenicePath.java b/services/venice-router/src/main/java/com/linkedin/venice/router/api/path/VenicePath.java
@@ -31,7 +31,6 @@ public abstract class VenicePath implements ResourcePath<RouterKey> {
   private Collection<RouterKey> partitionKeys;
   protected final RouterRetryConfig retryConfig;
   protected final RetryManager retryManager;
-  private final Time time;
   private final VeniceResponseDecompressor responseDecompressor;
   private boolean retryRequest = false;
   private long originalRequestStartTs = -1;
@@ -53,17 +52,7 @@ public VenicePath(
       RouterRetryConfig retryConfig,
       RetryManager retryManager,
       VeniceResponseDecompressor responseDecompressor) {
-    this(storeVersionName, SystemTime.INSTANCE, retryConfig, retryManager, responseDecompressor);
-  }
-
-  public VenicePath(
-      StoreVersionName storeVersionName,
-      Time time,
-      RouterRetryConfig retryConfig,
-      RetryManager retryManager,
-      VeniceResponseDecompressor responseDecompressor) {
     this.storeVersionName = storeVersionName;
-    this.time = time;
     this.retryConfig = retryConfig;
     this.retryManager = retryManager;
     this.responseDecompressor = responseDecompressor;
@@ -199,7 +188,7 @@ public boolean canRequestStorageNode(String storageNode) {
 
   public void recordOriginalRequestStartTimestamp() {
     if (!isRetryRequest()) {
-      setOriginalRequestStartTs(time.getMilliseconds());
+      setOriginalRequestStartTs(getTime().getMilliseconds());
     }
   }
 
@@ -214,7 +203,7 @@ public boolean isRetryRequestTooLate() {
     }
     if (isRetryRequest()) {
       // Retry request
-      long retryDelay = time.getMilliseconds() - getOriginalRequestStartTs();
+      long retryDelay = getTime().getMilliseconds() - getOriginalRequestStartTs();
       long smartRetryThreshold = getLongTailRetryThresholdMs() + getSmartLongTailRetryAbortThresholdMs();
       if (retryDelay > smartRetryThreshold) {
         return true;
@@ -299,4 +288,8 @@ public void recordRequest() {
   public boolean isLongTailRetryWithinBudget(int numberOfRoutes) {
     return retryManager.isRetryAllowed(numberOfRoutes);
   }
+
+  protected Time getTime() {
+    return SystemTime.INSTANCE;
+  }
 }
diff --git a/services/venice-router/src/test/java/com/linkedin/venice/router/api/path/TestVenicePath.java b/services/venice-router/src/test/java/com/linkedin/venice/router/api/path/TestVenicePath.java
@@ -39,14 +39,11 @@ public class TestVenicePath {
   private static class SmartRetryVenicePath extends VenicePath {
     private final String ROUTER_REQUEST_VERSION =
         Integer.toString(ReadAvroProtocolDefinition.SINGLE_GET_ROUTER_REQUEST_V1.getProtocolVersion());
+    private final Time time;
 
     public SmartRetryVenicePath(Time time, RetryManager retryManager) {
-      super(
-          nameRepository.getStoreVersionName("fake_resource_v1"),
-          time,
-          mock(RouterRetryConfig.class),
-          retryManager,
-          null);
+      super(nameRepository.getStoreVersionName("fake_resource_v1"), mock(RouterRetryConfig.class), retryManager, null);
+      this.time = time;
     }
 
     @Override
@@ -103,6 +100,11 @@ public int getSmartLongTailRetryAbortThresholdMs() {
     public int getLongTailRetryThresholdMs() {
       return 20;
     }
+
+    @Override
+    protected Time getTime() {
+      return this.time;
+    }
   }
 
   private RetryManager disabledRetryManager;