#19900

Committed 09 Jul 2025 02:53PM UTC coverage: 88.528% (-0.001%) from 88.529%

Build # #19900

Build Type

push

github

Committed by

ejona86

Commit Message

Fix RLS regressions from XdsDepMan conversion

297ab05ef converted CDS to XdsDependencyManager. This caused three
regressions:

 * CdsLB2 as a RLS child would always fail with "Unable to find
   non-dynamic root cluster" because is_dynamic=true was missing in
   its service config
 * XdsNameResolver only propagated resolution updates when the clusters
   changed, so a CdsUpdate change would be ignored. This caused a hang
   for RLS even with is_dynamic=true. For non-RLS the lack config update
   broke the circuit breaking psm interop test. This would have been
   more severe if ClusterResolverLb had been converted to
   XdsDependenceManager, as it would have ignored EDS updates
 * RLS did not propagate resolution updates, so CdsLB2 even with
   is_dynamic=true the CdsUpdate for the new cluster would never arrive,
   causing a hang

b/428120265
b/427912384

Run Details

34649 of 39139 relevant lines covered (88.53%)

0.89 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

84.31

/../rls/src/main/java/io/grpc/rls/RlsLoadBalancer.java

/*
 * Copyright 2020 The gRPC Authors
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package io.grpc.rls;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import io.grpc.ChannelLogger;
import io.grpc.ChannelLogger.ChannelLogLevel;
import io.grpc.ConnectivityState;
import io.grpc.LoadBalancer;
import io.grpc.Status;
import javax.annotation.Nullable;

/**
 * Implementation of {@link LoadBalancer} backed by route lookup service.
 */
final class RlsLoadBalancer extends LoadBalancer {

  private final ChannelLogger logger;
  private final Helper helper;
  @VisibleForTesting
  CachingRlsLbClientBuilderProvider cachingRlsLbClientBuilderProvider =
      new DefaultCachingRlsLbClientBuilderProvider();
  @Nullable
  private LbPolicyConfiguration lbPolicyConfiguration;
  @Nullable
  private CachingRlsLbClient routeLookupClient;

  RlsLoadBalancer(Helper helper) {
    this.helper = checkNotNull(helper, "helper");
    logger = helper.getChannelLogger();
    logger.log(ChannelLogLevel.DEBUG, "Rls lb created. Authority: {0}", helper.getAuthority());
  }

  @Override
  public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
    LbPolicyConfiguration lbPolicyConfiguration =
        (LbPolicyConfiguration) resolvedAddresses.getLoadBalancingPolicyConfig();
    checkNotNull(lbPolicyConfiguration, "Missing RLS LB config");
    if (!lbPolicyConfiguration.equals(this.lbPolicyConfiguration)) {
      logger.log(ChannelLogLevel.DEBUG, "A new RLS LB config received: {0}", lbPolicyConfiguration);
      boolean needToConnect = this.lbPolicyConfiguration == null
          || !this.lbPolicyConfiguration.getRouteLookupConfig().lookupService().equals(
          lbPolicyConfiguration.getRouteLookupConfig().lookupService());
      if (needToConnect) {
        logger.log(ChannelLogLevel.DEBUG, "RLS lookup service changed, need to connect");
        if (routeLookupClient != null) {
          routeLookupClient.close();
        }
        routeLookupClient =
            cachingRlsLbClientBuilderProvider
                .get()
                .setHelper(helper)
                .setLbPolicyConfig(lbPolicyConfiguration)
                .setResolvedAddressesFactory(
                    new ChildLbResolvedAddressFactory(
                        resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()))
                .build();
        logger.log(
            ChannelLogLevel.DEBUG, "LbPolicyConfiguration updated to {0}", lbPolicyConfiguration);
      }
      // TODO(creamsoup) allow incremental service config update. for initial use case, it is 
      //  not required.
      this.lbPolicyConfiguration = lbPolicyConfiguration;
    }
    return routeLookupClient.acceptResolvedAddressFactory(
        new ChildLbResolvedAddressFactory(
            resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()));
  }

  @Override
  public void requestConnection() {
    if (routeLookupClient != null) {
      routeLookupClient.requestConnection();
    }
  }

  @Override
  public void handleNameResolutionError(final Status error) {
    class ErrorPicker extends SubchannelPicker {
      @Override
      public PickResult pickSubchannel(PickSubchannelArgs args) {
        return PickResult.withError(error);
      }

      @Override
      public String toString() {
        return MoreObjects.toStringHelper(this)
            .add("error", error)
            .toString();
      }
    }

    if (routeLookupClient != null) {
      logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient on a name resolution error");
      routeLookupClient.close();
      routeLookupClient = null;
      lbPolicyConfiguration = null;
    }
    helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, new ErrorPicker());
  }

  @Override
  public void shutdown() {
    if (routeLookupClient != null) {
      logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient because of RLS LB shutdown");
      routeLookupClient.close();
      routeLookupClient = null;
    }
  }

  /**
   * Provides {@link CachingRlsLbClient.Builder} with default settings. This is useful for
   * testing.
   */
  interface CachingRlsLbClientBuilderProvider {
    CachingRlsLbClient.Builder get();
  }

  static final class DefaultCachingRlsLbClientBuilderProvider
      implements CachingRlsLbClientBuilderProvider {

    @Override
    public CachingRlsLbClient.Builder get() {
      return CachingRlsLbClient.newBuilder().setThrottler(AdaptiveThrottler.builder().build());
    }
  }
}

1	/*
2	* Copyright 2020 The gRPC Authors
3	*
4	* Licensed under the Apache License, Version 2.0 (the "License");
5	* you may not use this file except in compliance with the License.
6	* You may obtain a copy of the License at
7	*
8	* http://www.apache.org/licenses/LICENSE-2.0
9	*
10	* Unless required by applicable law or agreed to in writing, software
11	* distributed under the License is distributed on an "AS IS" BASIS,
12	* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13	* See the License for the specific language governing permissions and
14	* limitations under the License.
15	*/
16
17	package io.grpc.rls;
18
19	import static com.google.common.base.Preconditions.checkNotNull;
20
21	import com.google.common.annotations.VisibleForTesting;
22	import com.google.common.base.MoreObjects;
23	import io.grpc.ChannelLogger;
24	import io.grpc.ChannelLogger.ChannelLogLevel;
25	import io.grpc.ConnectivityState;
26	import io.grpc.LoadBalancer;
27	import io.grpc.Status;
28	import javax.annotation.Nullable;
29
30	/**
31	* Implementation of {@link LoadBalancer} backed by route lookup service.
32	*/
33	final class RlsLoadBalancer extends LoadBalancer {
34
35	private final ChannelLogger logger;
36	private final Helper helper;
37	@VisibleForTesting	1✔
38	CachingRlsLbClientBuilderProvider cachingRlsLbClientBuilderProvider =
39	new DefaultCachingRlsLbClientBuilderProvider();
40	@Nullable
41	private LbPolicyConfiguration lbPolicyConfiguration;
42	@Nullable
43	private CachingRlsLbClient routeLookupClient;
44
45	RlsLoadBalancer(Helper helper) {	1✔
46	this.helper = checkNotNull(helper, "helper");	1✔
47	logger = helper.getChannelLogger();	1✔
48	logger.log(ChannelLogLevel.DEBUG, "Rls lb created. Authority: {0}", helper.getAuthority());	1✔
49	}	1✔
50
51	@Override
52	public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
53	LbPolicyConfiguration lbPolicyConfiguration =	1✔
54	(LbPolicyConfiguration) resolvedAddresses.getLoadBalancingPolicyConfig();	1✔
55	checkNotNull(lbPolicyConfiguration, "Missing RLS LB config");	1✔
56	if (!lbPolicyConfiguration.equals(this.lbPolicyConfiguration)) {	1✔
57	logger.log(ChannelLogLevel.DEBUG, "A new RLS LB config received: {0}", lbPolicyConfiguration);	1✔
58	boolean needToConnect = this.lbPolicyConfiguration == null	1✔
59	\|\| !this.lbPolicyConfiguration.getRouteLookupConfig().lookupService().equals(	1✔
60	lbPolicyConfiguration.getRouteLookupConfig().lookupService());	×
61	if (needToConnect) {	1✔
62	logger.log(ChannelLogLevel.DEBUG, "RLS lookup service changed, need to connect");	1✔
63	if (routeLookupClient != null) {	1✔
64	routeLookupClient.close();	×
65	}
66	routeLookupClient =	1✔
67	cachingRlsLbClientBuilderProvider
68	.get()	1✔
69	.setHelper(helper)	1✔
70	.setLbPolicyConfig(lbPolicyConfiguration)	1✔
71	.setResolvedAddressesFactory(	1✔
72	new ChildLbResolvedAddressFactory(
73	resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()))	1✔
74	.build();	1✔
75	logger.log(	1✔
76	ChannelLogLevel.DEBUG, "LbPolicyConfiguration updated to {0}", lbPolicyConfiguration);
77	}
78	// TODO(creamsoup) allow incremental service config update. for initial use case, it is
79	// not required.
80	this.lbPolicyConfiguration = lbPolicyConfiguration;	1✔
81	}
82	return routeLookupClient.acceptResolvedAddressFactory(	1✔
83	new ChildLbResolvedAddressFactory(
84	resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()));	1✔
85	}
86
87	@Override
88	public void requestConnection() {
89	if (routeLookupClient != null) {	×
90	routeLookupClient.requestConnection();	×
91	}
92	}	×
93
94	@Override
95	public void handleNameResolutionError(final Status error) {
96	class ErrorPicker extends SubchannelPicker {	1✔
97	@Override
98	public PickResult pickSubchannel(PickSubchannelArgs args) {
99	return PickResult.withError(error);	1✔
100	}
101
102	@Override
103	public String toString() {
104	return MoreObjects.toStringHelper(this)	×
105	.add("error", error)	×
106	.toString();	×
107	}
108	}
109
110	if (routeLookupClient != null) {	1✔
111	logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient on a name resolution error");	1✔
112	routeLookupClient.close();	1✔
113	routeLookupClient = null;	1✔
114	lbPolicyConfiguration = null;	1✔
115	}
116	helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, new ErrorPicker());	1✔
117	}	1✔
118
119	@Override
120	public void shutdown() {
121	if (routeLookupClient != null) {	1✔
122	logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient because of RLS LB shutdown");	1✔
123	routeLookupClient.close();	1✔
124	routeLookupClient = null;	1✔
125	}
126	}	1✔
127
128	/**
129	* Provides {@link CachingRlsLbClient.Builder} with default settings. This is useful for
130	* testing.
131	*/
132	interface CachingRlsLbClientBuilderProvider {
133	CachingRlsLbClient.Builder get();
134	}
135
136	static final class DefaultCachingRlsLbClientBuilderProvider	1✔
137	implements CachingRlsLbClientBuilderProvider {
138
139	@Override
140	public CachingRlsLbClient.Builder get() {
141	return CachingRlsLbClient.newBuilder().setThrottler(AdaptiveThrottler.builder().build());	1✔
142	}
143	}
144	}

grpc / grpc-java / #19900

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous