Resolve comments

CHANGELOG.md

@@ -14,7 +14,7 @@ and what APIs have changed, if applicable.
 
 ## [Unreleased]
 
-## [29.46.10] - 2023-11-13
+## [29.47.0] - 2023-11-13
 Fix dual-read potential risk that newLb may impact oldLb
 
 ## [29.46.9] - 2023-11-02
@@ -5560,8 +5560,8 @@ patch operations can re-use these classes for generating patch messages.
 
 ## [0.14.1]
 
-[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.46.10...master
-[29.46.10]: https://github.com/linkedin/rest.li/compare/v29.46.9...v29.46.10
+[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.47.0...master
+[29.47.0]: https://github.com/linkedin/rest.li/compare/v29.46.9...v29.47.0
 [29.46.9]: https://github.com/linkedin/rest.li/compare/v29.46.8...v29.46.9
 [29.46.8]: https://github.com/linkedin/rest.li/compare/v29.46.7...v29.46.8
 [29.46.7]: https://github.com/linkedin/rest.li/compare/v29.46.6...v29.46.7

d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java

@@ -202,7 +202,7 @@ public D2Client build()
                   _config.dualReadStateManager,
                   _config.xdsExecutorService,
                   _config.xdsStreamReadyTimeout,
-                  _config.loadBalancerExecutor
+                  _config.dualReadNewLbExecutor
     );
 
     final LoadBalancerWithFacilitiesFactory loadBalancerFactory = (_config.lbWithFacilitiesFactory == null) ?
@@ -644,8 +644,8 @@ public D2ClientBuilder setDualReadStateManager(DualReadStateManager dualReadStat
     return this;
   }
 
-  public D2ClientBuilder setLoadBalancerExecutor(ExecutorService loadBalancerExecutor) {
-    _config.loadBalancerExecutor = loadBalancerExecutor;
+  public D2ClientBuilder setDualReadNewLbExecutor(ExecutorService dualReadNewLbExecutor) {
+    _config.dualReadNewLbExecutor = dualReadNewLbExecutor;
     return this;
   }
 

d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java

@@ -128,7 +128,7 @@ public class D2ClientConfig
 
   public ScheduledExecutorService xdsExecutorService = null;
   public Long xdsStreamReadyTimeout = null;
-  public ExecutorService loadBalancerExecutor = null;
+  public ExecutorService dualReadNewLbExecutor = null;
 
   public D2ClientConfig()
   {
@@ -198,7 +198,7 @@ public D2ClientConfig()
                  DualReadStateManager dualReadStateManager,
                  ScheduledExecutorService xdsExecutorService,
                  Long xdsStreamReadyTimeout,
-                 ExecutorService loadBalancerExecutor)
+                 ExecutorService dualReadNewLbExecutor)
   {
     this.zkHosts = zkHosts;
     this.xdsServer = xdsServer;
@@ -264,6 +264,6 @@ public D2ClientConfig()
     this.dualReadStateManager = dualReadStateManager;
     this.xdsExecutorService = xdsExecutorService;
     this.xdsStreamReadyTimeout = xdsStreamReadyTimeout;
-    this.loadBalancerExecutor = loadBalancerExecutor;
+    this.dualReadNewLbExecutor = dualReadNewLbExecutor;
   }
 }

d2/src/main/java/com/linkedin/d2/balancer/dualread/DualReadLoadBalancer.java

@@ -61,7 +61,7 @@ public class DualReadLoadBalancer implements LoadBalancerWithFacilities
   private final LoadBalancerWithFacilities _oldLb;
   private final LoadBalancerWithFacilities _newLb;
   private final DualReadStateManager _dualReadStateManager;
-  private ExecutorService _executor;
+  private ExecutorService _newLbExecutor;
   private boolean _isNewLbReady;
 
   @Deprecated
@@ -73,22 +73,22 @@ public DualReadLoadBalancer(LoadBalancerWithFacilities oldLb, LoadBalancerWithFa
   }
 
   public DualReadLoadBalancer(LoadBalancerWithFacilities oldLb, LoadBalancerWithFacilities newLb,
-      @Nonnull DualReadStateManager dualReadStateManager, ExecutorService executor)
+      @Nonnull DualReadStateManager dualReadStateManager, ExecutorService newLbExecutor)
   {
     _oldLb = oldLb;
     _newLb = newLb;
     _dualReadStateManager = dualReadStateManager;
     _isNewLbReady = false;
-    if(executor == null)
+    if(newLbExecutor == null)
     {
       // Using a direct executor here means the code is executed directly,
       // blocking the caller. This means the old behavior is preserved.
-      _executor = MoreExecutors.newDirectExecutorService();
-      LOG.warn("Deprecated DualReadLoadBalancer constructor used without a threadpool executor");
+      _newLbExecutor = MoreExecutors.newDirectExecutorService();
+      LOG.warn("The newLbExecutor is null, will use a direct executor instead.");
     }
     else
     {
-      _executor = executor;
+      _newLbExecutor = newLbExecutor;
     }
   }
 
@@ -130,13 +130,13 @@ public void getClient(Request request, RequestContext requestContext, Callback<T
         _newLb.getClient(request, requestContext, clientCallback);
         break;
       case DUAL_READ:
-        _executor.execute(
+        _newLbExecutor.execute(
             () -> _newLb.getLoadBalancedServiceProperties(serviceName, new Callback<ServiceProperties>()
             {
               @Override
               public void onError(Throwable e)
               {
-                LOG.error("Double read failure. Unable to read service properties from: {}", serviceName, e);
+                LOG.error("Dual read failure. Unable to read service properties from: {}", serviceName, e);
               }
 
               @Override
@@ -177,7 +177,7 @@ public void getLoadBalancedServiceProperties(String serviceName, Callback<Servic
         _newLb.getLoadBalancedServiceProperties(serviceName, clientCallback);
         break;
       case DUAL_READ:
-        _executor.execute(() -> _newLb.getLoadBalancedServiceProperties(serviceName, Callbacks.empty()));
+        _newLbExecutor.execute(() -> _newLb.getLoadBalancedServiceProperties(serviceName, Callbacks.empty()));
         _oldLb.getLoadBalancedServiceProperties(serviceName, clientCallback);
         break;
       case OLD_LB_ONLY:
@@ -196,7 +196,7 @@ public void getLoadBalancedClusterAndUriProperties(String clusterName,
         _newLb.getLoadBalancedClusterAndUriProperties(clusterName, callback);
         break;
       case DUAL_READ:
-        _executor.execute(() -> _newLb.getLoadBalancedClusterAndUriProperties(clusterName, Callbacks.empty()));
+        _newLbExecutor.execute(() -> _newLb.getLoadBalancedClusterAndUriProperties(clusterName, Callbacks.empty()));
         _oldLb.getLoadBalancedClusterAndUriProperties(clusterName, callback);
         break;
       case OLD_LB_ONLY:

d2/src/main/java/com/linkedin/d2/xds/balancer/DualReadZkAndXdsLoadBalancerFactory.java

@@ -44,6 +44,6 @@ public DualReadZkAndXdsLoadBalancerFactory(@Nonnull DualReadStateManager dualRea
   @Override
   public LoadBalancerWithFacilities create(D2ClientConfig config)
   {
-    return new DualReadLoadBalancer(_zkLbFactory.create(config), _xdsLbFactory.create(config), _dualReadStateManager, config.loadBalancerExecutor);
+    return new DualReadLoadBalancer(_zkLbFactory.create(config), _xdsLbFactory.create(config), _dualReadStateManager, config.dualReadNewLbExecutor);
   }
 }

gradle.properties

@@ -1,4 +1,4 @@
-version=29.46.10
+version=29.47.0
 group=com.linkedin.pegasus
 org.gradle.configureondemand=true
 org.gradle.parallel=true