Skip to content

Commit 7308d92

Browse files
authored
rls: support routeLookupChannelServiceConfig in RLS lb config
Implementing the latest change for RLS lb config. ``` The configuration for the LB policy will be of the following form: { "routeLookupConfig": <JSON form of RouteLookupConfig proto>, "routeLookupChannelServiceConfig": {...service config JSON...}, "childPolicy": [ {"<policy name>": {...child policy config...}} ], "childPolicyConfigTargetFieldName": "<name of field>" } ``` >If the routeLookupChannelServiceConfig field is present, we will pass the specified service config to the RLS control plane channel, and we will disable fetching service config via that channel's resolver.
1 parent bd156f9 commit 7308d92

File tree

5 files changed

+48
-61
lines changed

5 files changed

+48
-61
lines changed

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

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
import com.google.common.base.Converter;
2424
import com.google.common.base.MoreObjects;
2525
import com.google.common.base.MoreObjects.ToStringHelper;
26-
import com.google.common.collect.ImmutableList;
27-
import com.google.common.collect.ImmutableMap;
2826
import com.google.common.util.concurrent.ListenableFuture;
2927
import com.google.common.util.concurrent.SettableFuture;
3028
import io.grpc.ChannelLogger;
@@ -83,13 +81,6 @@ final class CachingRlsLbClient {
8381
private static final Converter<RouteLookupResponse, io.grpc.lookup.v1.RouteLookupResponse>
8482
RESPONSE_CONVERTER = new RouteLookupResponseConverter().reverse();
8583

86-
// System property to use direct path enabled OobChannel, by default direct path is enabled.
87-
private static final String RLS_ENABLE_OOB_CHANNEL_DIRECTPATH_PROPERTY =
88-
"io.grpc.rls.CachingRlsLbClient.enable_oobchannel_directpath";
89-
@VisibleForTesting
90-
static boolean enableOobChannelDirectPath =
91-
Boolean.parseBoolean(System.getProperty(RLS_ENABLE_OOB_CHANNEL_DIRECTPATH_PROPERTY, "false"));
92-
9384
// All cache status changes (pending, backoff, success) must be under this lock
9485
private final Object lock = new Object();
9586
// LRU cache based on access order (BACKOFF and actual data will be here)
@@ -158,14 +149,14 @@ private CachingRlsLbClient(Builder builder) {
158149
ManagedChannelBuilder<?> rlsChannelBuilder = helper.createResolvingOobChannelBuilder(
159150
rlsConfig.getLookupService(), helper.getUnsafeChannelCredentials());
160151
rlsChannelBuilder.overrideAuthority(helper.getAuthority());
161-
if (enableOobChannelDirectPath) {
162-
Map<String, ?> directPathServiceConfig =
163-
getDirectPathServiceConfig(rlsConfig.getLookupService());
152+
Map<String, ?> routeLookupChannelServiceConfig =
153+
lbPolicyConfig.getRouteLookupChannelServiceConfig();
154+
if (routeLookupChannelServiceConfig != null) {
164155
logger.log(
165156
ChannelLogLevel.DEBUG,
166-
"RLS channel direct path enabled. RLS channel service config: {0}",
167-
directPathServiceConfig);
168-
rlsChannelBuilder.defaultServiceConfig(directPathServiceConfig);
157+
"RLS channel service config: {0}",
158+
routeLookupChannelServiceConfig);
159+
rlsChannelBuilder.defaultServiceConfig(routeLookupChannelServiceConfig);
169160
rlsChannelBuilder.disableServiceConfigLookUp();
170161
}
171162
rlsChannel = rlsChannelBuilder.build();
@@ -184,21 +175,6 @@ private CachingRlsLbClient(Builder builder) {
184175
logger.log(ChannelLogLevel.DEBUG, "CachingRlsLbClient created");
185176
}
186177

187-
private static ImmutableMap<String, Object> getDirectPathServiceConfig(String serviceName) {
188-
ImmutableMap<String, Object> pickFirstStrategy =
189-
ImmutableMap.<String, Object>of("pick_first", ImmutableMap.of());
190-
191-
ImmutableMap<String, Object> childPolicy =
192-
ImmutableMap.<String, Object>of(
193-
"childPolicy", ImmutableList.of(pickFirstStrategy),
194-
"serviceName", serviceName);
195-
196-
ImmutableMap<String, Object> grpcLbPolicy =
197-
ImmutableMap.<String, Object>of("grpclb", childPolicy);
198-
199-
return ImmutableMap.<String, Object>of("loadBalancingConfig", ImmutableList.of(grpcLbPolicy));
200-
}
201-
202178
@CheckReturnValue
203179
private ListenableFuture<RouteLookupResponse> asyncRlsCall(RouteLookupRequest request) {
204180
final SettableFuture<RouteLookupResponse> response = SettableFuture.create();

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,27 @@
4848
final class LbPolicyConfiguration {
4949

5050
private final RouteLookupConfig routeLookupConfig;
51+
@Nullable
52+
private final Map<String, ?> routeLookupChannelServiceConfig;
5153
private final ChildLoadBalancingPolicy policy;
5254

5355
LbPolicyConfiguration(
54-
RouteLookupConfig routeLookupConfig, ChildLoadBalancingPolicy policy) {
56+
RouteLookupConfig routeLookupConfig, @Nullable Map<String, ?> routeLookupChannelServiceConfig,
57+
ChildLoadBalancingPolicy policy) {
5558
this.routeLookupConfig = checkNotNull(routeLookupConfig, "routeLookupConfig");
59+
this.routeLookupChannelServiceConfig = routeLookupChannelServiceConfig;
5660
this.policy = checkNotNull(policy, "policy");
5761
}
5862

5963
RouteLookupConfig getRouteLookupConfig() {
6064
return routeLookupConfig;
6165
}
6266

67+
@Nullable
68+
Map<String, ?> getRouteLookupChannelServiceConfig() {
69+
return routeLookupChannelServiceConfig;
70+
}
71+
6372
ChildLoadBalancingPolicy getLoadBalancingPolicy() {
6473
return policy;
6574
}
@@ -74,18 +83,20 @@ public boolean equals(Object o) {
7483
}
7584
LbPolicyConfiguration that = (LbPolicyConfiguration) o;
7685
return Objects.equals(routeLookupConfig, that.routeLookupConfig)
86+
&& Objects.equals(routeLookupChannelServiceConfig, that.routeLookupChannelServiceConfig)
7787
&& Objects.equals(policy, that.policy);
7888
}
7989

8090
@Override
8191
public int hashCode() {
82-
return Objects.hash(routeLookupConfig, policy);
92+
return Objects.hash(routeLookupConfig, routeLookupChannelServiceConfig, policy);
8393
}
8494

8595
@Override
8696
public String toString() {
8797
return MoreObjects.toStringHelper(this)
8898
.add("routeLookupConfig", routeLookupConfig)
99+
.add("routeLookupChannelServiceConfig", routeLookupChannelServiceConfig)
89100
.add("policy", policy)
90101
.toString();
91102
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,15 @@ public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawLoadBalanc
6262
try {
6363
RouteLookupConfig routeLookupConfig = new RouteLookupConfigConverter()
6464
.convert(JsonUtil.getObject(rawLoadBalancingConfigPolicy, "routeLookupConfig"));
65+
Map<String, ?> routeLookupChannelServiceConfig =
66+
JsonUtil.getObject(rawLoadBalancingConfigPolicy, "routeLookupChannelServiceConfig");
6567
ChildLoadBalancingPolicy lbPolicy = ChildLoadBalancingPolicy
6668
.create(
6769
JsonUtil.getString(rawLoadBalancingConfigPolicy, "childPolicyConfigTargetFieldName"),
6870
JsonUtil.checkObjectList(
6971
checkNotNull(JsonUtil.getList(rawLoadBalancingConfigPolicy, "childPolicy"))));
70-
return ConfigOrError.fromConfig(new LbPolicyConfiguration(routeLookupConfig, lbPolicy));
72+
return ConfigOrError.fromConfig(
73+
new LbPolicyConfiguration(routeLookupConfig, routeLookupChannelServiceConfig, lbPolicy));
7174
} catch (Exception e) {
7275
return ConfigOrError.fromError(
7376
Status.INVALID_ARGUMENT

rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@
9090
import java.util.concurrent.TimeoutException;
9191
import javax.annotation.Nonnull;
9292
import org.junit.After;
93-
import org.junit.Before;
9493
import org.junit.Rule;
9594
import org.junit.Test;
9695
import org.junit.runner.RunWith;
@@ -144,19 +143,12 @@ public void uncaughtException(Thread t, Throwable e) {
144143
mock(Helper.class, AdditionalAnswers.delegatesTo(new FakeHelper()));
145144
private final FakeThrottler fakeThrottler = new FakeThrottler();
146145
private final LbPolicyConfiguration lbPolicyConfiguration =
147-
new LbPolicyConfiguration(ROUTE_LOOKUP_CONFIG, childLbPolicy);
146+
new LbPolicyConfiguration(ROUTE_LOOKUP_CONFIG, null, childLbPolicy);
148147

149148
private CachingRlsLbClient rlsLbClient;
150-
private boolean existingEnableOobChannelDirectPath;
151149
private Map<String, ?> rlsChannelServiceConfig;
152150
private String rlsChannelOverriddenAuthority;
153151

154-
@Before
155-
public void setUp() throws Exception {
156-
existingEnableOobChannelDirectPath = CachingRlsLbClient.enableOobChannelDirectPath;
157-
CachingRlsLbClient.enableOobChannelDirectPath = false;
158-
}
159-
160152
private void setUpRlsLbClient() {
161153
rlsLbClient =
162154
CachingRlsLbClient.newBuilder()
@@ -173,7 +165,6 @@ private void setUpRlsLbClient() {
173165
@After
174166
public void tearDown() throws Exception {
175167
rlsLbClient.close();
176-
CachingRlsLbClient.enableOobChannelDirectPath = existingEnableOobChannelDirectPath;
177168
assertWithMessage(
178169
"On client shut down, RlsLoadBalancer must shut down with all its child loadbalancers.")
179170
.that(lbProvider.loadBalancers).isEmpty();
@@ -244,9 +235,29 @@ public void get_noError_lifeCycle() throws Exception {
244235
}
245236

246237
@Test
247-
public void rls_overDirectPath() throws Exception {
248-
CachingRlsLbClient.enableOobChannelDirectPath = true;
249-
setUpRlsLbClient();
238+
public void rls_withCustomRlsChannelServiceConfig() throws Exception {
239+
Map<String, ?> routeLookupChannelServiceConfig =
240+
ImmutableMap.of(
241+
"loadBalancingConfig",
242+
ImmutableList.of(ImmutableMap.of(
243+
"grpclb",
244+
ImmutableMap.of(
245+
"childPolicy",
246+
ImmutableList.of(ImmutableMap.of("pick_first", ImmutableMap.of())),
247+
"serviceName",
248+
"service1"))));
249+
LbPolicyConfiguration lbPolicyConfiguration = new LbPolicyConfiguration(
250+
ROUTE_LOOKUP_CONFIG, routeLookupChannelServiceConfig, childLbPolicy);
251+
rlsLbClient =
252+
CachingRlsLbClient.newBuilder()
253+
.setBackoffProvider(fakeBackoffProvider)
254+
.setResolvedAddressesFactory(resolvedAddressFactory)
255+
.setEvictionListener(evictionListener)
256+
.setHelper(helper)
257+
.setLbPolicyConfig(lbPolicyConfiguration)
258+
.setThrottler(fakeThrottler)
259+
.setTimeProvider(fakeTimeProvider)
260+
.build();
250261
RouteLookupRequest routeLookupRequest = new RouteLookupRequest(ImmutableMap.of(
251262
"server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar"));
252263
rlsServerImpl.setLookupTable(
@@ -265,16 +276,7 @@ public void rls_overDirectPath() throws Exception {
265276
assertThat(resp.hasData()).isTrue();
266277

267278
assertThat(rlsChannelOverriddenAuthority).isEqualTo("bigtable.googleapis.com:443");
268-
assertThat(rlsChannelServiceConfig).isEqualTo(
269-
ImmutableMap.of(
270-
"loadBalancingConfig",
271-
ImmutableList.of(ImmutableMap.of(
272-
"grpclb",
273-
ImmutableMap.of(
274-
"childPolicy",
275-
ImmutableList.of(ImmutableMap.of("pick_first", ImmutableMap.of())),
276-
"serviceName",
277-
"service1")))));
279+
assertThat(rlsChannelServiceConfig).isEqualTo(routeLookupChannelServiceConfig);
278280
}
279281

280282
@Test

rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,16 +118,12 @@ public void uncaughtException(Thread t, Throwable e) {
118118
private MethodDescriptor<Object, Object> fakeSearchMethod;
119119
private MethodDescriptor<Object, Object> fakeRescueMethod;
120120
private RlsLoadBalancer rlsLb;
121-
private boolean existingEnableOobChannelDirectPath;
122121
private String defaultTarget = "defaultTarget";
123122

124123
@Before
125124
public void setUp() throws Exception {
126125
MockitoAnnotations.initMocks(this);
127126

128-
existingEnableOobChannelDirectPath = CachingRlsLbClient.enableOobChannelDirectPath;
129-
CachingRlsLbClient.enableOobChannelDirectPath = false;
130-
131127
fakeSearchMethod =
132128
MethodDescriptor.newBuilder()
133129
.setFullMethodName("com.google/Search")
@@ -168,7 +164,6 @@ public CachingRlsLbClient.Builder get() {
168164
@After
169165
public void tearDown() throws Exception {
170166
rlsLb.shutdown();
171-
CachingRlsLbClient.enableOobChannelDirectPath = existingEnableOobChannelDirectPath;
172167
}
173168

174169
@Test

0 commit comments

Comments
 (0)