From 1dd9d1b05260a8124587da21b25b89c11d763659 Mon Sep 17 00:00:00 2001 From: brycezhongqing Date: Mon, 13 Nov 2023 17:24:06 -0800 Subject: [PATCH] Resolve comments --- CHANGELOG.md | 6 +++--- .../linkedin/d2/balancer/D2ClientBuilder.java | 6 +++--- .../linkedin/d2/balancer/D2ClientConfig.java | 6 +++--- .../dualread/DualReadLoadBalancer.java | 20 +++++++++---------- .../DualReadZkAndXdsLoadBalancerFactory.java | 2 +- gradle.properties | 2 +- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80d9a9ed07..428fdd9fbc 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java index 7d246bbe20..ea4838c15c 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java +++ b/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; } diff --git a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java index 5734c133d0..8f7fc44b52 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java +++ b/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; } } diff --git a/d2/src/main/java/com/linkedin/d2/balancer/dualread/DualReadLoadBalancer.java b/d2/src/main/java/com/linkedin/d2/balancer/dualread/DualReadLoadBalancer.java index 970e804dbe..bfe387f3bd 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/dualread/DualReadLoadBalancer.java +++ b/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 _newLb.getLoadBalancedServiceProperties(serviceName, new Callback() { @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 _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: diff --git a/d2/src/main/java/com/linkedin/d2/xds/balancer/DualReadZkAndXdsLoadBalancerFactory.java b/d2/src/main/java/com/linkedin/d2/xds/balancer/DualReadZkAndXdsLoadBalancerFactory.java index 024e9c02c1..5f96357144 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/balancer/DualReadZkAndXdsLoadBalancerFactory.java +++ b/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); } } \ No newline at end of file diff --git a/gradle.properties b/gradle.properties index 02c6ef69fd..567d162c59 100644 --- a/gradle.properties +++ b/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