Skip to content

Commit 409418b

Browse files
authored
Merge pull request #2476 from murgatroid99/grpc-js_prohibit_od_pick_first
grpc-js: Disallow pick_first as child of outlier_detection
2 parents 073caf5 + ed70a0b commit 409418b

File tree

4 files changed

+21
-8
lines changed

4 files changed

+21
-8
lines changed

packages/grpc-js/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@grpc/grpc-js",
3-
"version": "1.8.16",
3+
"version": "1.8.17",
44
"description": "gRPC Library for Node - pure JS implementation",
55
"homepage": "https://grpc.io/",
66
"repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

packages/grpc-js/src/load-balancer-outlier-detection.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ export class OutlierDetectionLoadBalancingConfig implements LoadBalancingConfig
113113
failurePercentageEjection: Partial<FailurePercentageEjectionConfig> | null,
114114
private readonly childPolicy: LoadBalancingConfig[]
115115
) {
116+
if (childPolicy.length > 0 && childPolicy[0].getLoadBalancerName() === 'pick_first') {
117+
throw new Error('outlier_detection LB policy cannot have a pick_first child policy');
118+
}
116119
this.intervalMs = intervalMs ?? 10_000;
117120
this.baseEjectionTimeMs = baseEjectionTimeMs ?? 30_000;
118121
this.maxEjectionTimeMs = maxEjectionTimeMs ?? 300_000;
@@ -395,8 +398,8 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
395398
}
396399

397400
private isCountingEnabled(): boolean {
398-
return this.latestConfig !== null &&
399-
(this.latestConfig.getSuccessRateEjectionConfig() !== null ||
401+
return this.latestConfig !== null &&
402+
(this.latestConfig.getSuccessRateEjectionConfig() !== null ||
400403
this.latestConfig.getFailurePercentageEjectionConfig() !== null);
401404
}
402405

@@ -496,7 +499,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
496499
if (addressesWithTargetVolume < failurePercentageConfig.minimum_hosts) {
497500
return;
498501
}
499-
502+
500503
// Step 2
501504
for (const [address, mapEntry] of this.addressMap.entries()) {
502505
// Step 2.i
@@ -656,4 +659,4 @@ export function setup() {
656659
if (OUTLIER_DETECTION_ENABLED) {
657660
registerLoadBalancerType(TYPE_NAME, OutlierDetectionLoadBalancer, OutlierDetectionLoadBalancingConfig);
658661
}
659-
}
662+
}

packages/grpc-js/src/resolver-dns.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class DnsResolver implements Resolver {
137137
details: `Name resolution failed for target ${uriToString(this.target)}`,
138138
metadata: new Metadata(),
139139
};
140-
140+
141141
const backoffOptions: BackoffOptions = {
142142
initialDelay: channelOptions['grpc.initial_reconnect_backoff_ms'],
143143
maxDelay: channelOptions['grpc.max_reconnect_backoff_ms'],
@@ -276,7 +276,7 @@ class DnsResolver implements Resolver {
276276
} catch (err) {
277277
this.latestServiceConfigError = {
278278
code: Status.UNAVAILABLE,
279-
details: 'Parsing service config failed',
279+
details: `Parsing service config failed with error ${(err as Error).message}`,
280280
metadata: new Metadata(),
281281
};
282282
}

packages/grpc-js/test/test-outlier-detection.ts

+11-1
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,16 @@ describe('Outlier detection config validation', () => {
360360
}, /failure_percentage_ejection\.enforcement_percentage parse error: value out of range for percentage/);
361361
});
362362
});
363+
describe('child_policy', () => {
364+
it('Should reject a pick_first child_policy', () => {
365+
const loadBalancingConfig = {
366+
child_policy: [{pick_first: {}}]
367+
};
368+
assert.throws(() => {
369+
OutlierDetectionLoadBalancingConfig.createFromJson(loadBalancingConfig);
370+
}, /outlier_detection LB policy cannot have a pick_first child policy/);
371+
});
372+
});
363373
});
364374

365375
describe('Outlier detection', () => {
@@ -533,4 +543,4 @@ describe('Outlier detection', () => {
533543
})
534544
});
535545
});
536-
});
546+
});

0 commit comments

Comments
 (0)