From 92c9237fe667f41bdbbf0192ba3666e24ee922fb Mon Sep 17 00:00:00 2001 From: ZHANG Dapeng Date: Fri, 14 Feb 2020 11:11:15 -0800 Subject: [PATCH] xds: have ClusterEndpointsBalancer cancel endpoint watcher only during shutdown Previously when eds service name is changed, the old endpoint watcher is canceled immediately even it's in graceful switch period, so the old ClusterEndpointsBalancer won't receive any new updates. This behavior is not as good/clean as cancelling the old watch only once the old ClusterEndpointsBalancer is shutdown. --- .../java/io/grpc/xds/EdsLoadBalancer.java | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/EdsLoadBalancer.java b/xds/src/main/java/io/grpc/xds/EdsLoadBalancer.java index 640a12dc4b..30963c1eb3 100644 --- a/xds/src/main/java/io/grpc/xds/EdsLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/EdsLoadBalancer.java @@ -69,9 +69,6 @@ final class EdsLoadBalancer extends LoadBalancer { // Most recent XdsConfig. @Nullable private XdsConfig xdsConfig; - // Most recent EndpointWatcher. - @Nullable - private EndpointWatcher endpointWatcher; @Nullable private ObjectPool xdsClientPool; @Nullable @@ -214,11 +211,7 @@ final class EdsLoadBalancer extends LoadBalancer { new ClusterEndpointsBalancerFactory(clusterServiceName); switchingLoadBalancer.switchTo(clusterEndpointsLoadBalancerFactory); } - resolvedAddresses = resolvedAddresses.toBuilder() - .setLoadBalancingPolicyConfig(newXdsConfig) - .build(); switchingLoadBalancer.handleResolvedAddresses(resolvedAddresses); - this.xdsConfig = newXdsConfig; } @@ -263,16 +256,9 @@ final class EdsLoadBalancer extends LoadBalancer { */ private final class ClusterEndpointsBalancerFactory extends LoadBalancer.Factory { final String clusterServiceName; - @Nullable - final String oldClusterServiceName; ClusterEndpointsBalancerFactory(String clusterServiceName) { this.clusterServiceName = clusterServiceName; - if (xdsConfig != null) { - oldClusterServiceName = xdsConfig.edsServiceName; - } else { - oldClusterServiceName = null; - } } @Override @@ -314,11 +300,6 @@ final class EdsLoadBalancer extends LoadBalancer { endpointWatcher = new EndpointWatcherImpl(localityStore); xdsClient.watchEndpointData(clusterServiceName, endpointWatcher); - if (EdsLoadBalancer.this.endpointWatcher != null) { - xdsClient.cancelEndpointDataWatch( - oldClusterServiceName, EdsLoadBalancer.this.endpointWatcher); - } - EdsLoadBalancer.this.endpointWatcher = endpointWatcher; } // TODO(zddapeng): In handleResolvedAddresses() handle child policy change if any.