#19908

Committed 16 Jul 2025 07:54PM UTC coverage: 88.593% (+0.07%) from 88.528%

Build # #19908

Build Type

push

github

Committed by

ejona86

Commit Message

Revert "xds: Convert CdsLb to XdsDepManager"

This reverts commit 297ab05ef.

b/430347751 shows multiple concerning behaviors in the xDS stack with
the new A74 config update model. XdsDepManager and CdsLB2 still seem to
be working correctly, but the change is exacerbated issues in other
parts of the stack, like RingHashConfig not having equals fixed in
a8de9f07ab.

Revert only for the v1.74.x release, leaving it on master.

Run Details

34647 of 39108 relevant lines covered (88.59%)

0.89 hits per line

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

84.0

/../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 Status.OK;
  }

  @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 Status.OK;	1✔
83	}
84
85	@Override
86	public void requestConnection() {
87	if (routeLookupClient != null) {	×
88	routeLookupClient.requestConnection();	×
89	}
90	}	×
91
92	@Override
93	public void handleNameResolutionError(final Status error) {
94	class ErrorPicker extends SubchannelPicker {	1✔
95	@Override
96	public PickResult pickSubchannel(PickSubchannelArgs args) {
97	return PickResult.withError(error);	1✔
98	}
99
100	@Override
101	public String toString() {
102	return MoreObjects.toStringHelper(this)	×
103	.add("error", error)	×
104	.toString();	×
105	}
106	}
107
108	if (routeLookupClient != null) {	1✔
109	logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient on a name resolution error");	1✔
110	routeLookupClient.close();	1✔
111	routeLookupClient = null;	1✔
112	lbPolicyConfiguration = null;	1✔
113	}
114	helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, new ErrorPicker());	1✔
115	}	1✔
116
117	@Override
118	public void shutdown() {
119	if (routeLookupClient != null) {	1✔
120	logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient because of RLS LB shutdown");	1✔
121	routeLookupClient.close();	1✔
122	routeLookupClient = null;	1✔
123	}
124	}	1✔
125
126	/**
127	* Provides {@link CachingRlsLbClient.Builder} with default settings. This is useful for
128	* testing.
129	*/
130	interface CachingRlsLbClientBuilderProvider {
131	CachingRlsLbClient.Builder get();
132	}
133
134	static final class DefaultCachingRlsLbClientBuilderProvider	1✔
135	implements CachingRlsLbClientBuilderProvider {
136
137	@Override
138	public CachingRlsLbClient.Builder get() {
139	return CachingRlsLbClient.newBuilder().setThrottler(AdaptiveThrottler.builder().build());	1✔
140	}
141	}
142	}

grpc / grpc-java / #19908

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