Skip to content

Commit 83b6e60

Browse files
authored
Merge pull request #2845 from murgatroid99/grpc-js-xds_config_tears_fixes
grpc-js-xds: Fix a couple of bugs with the config tears change
2 parents 8a31431 + eee7030 commit 83b6e60

File tree

4 files changed

+33
-4
lines changed

4 files changed

+33
-4
lines changed

packages/grpc-js-xds/src/xds-dependency-manager.ts

+12-3
Original file line numberDiff line numberDiff line change
@@ -653,8 +653,9 @@ export class XdsDependencyManager {
653653
this.subscribedClusters[clusterName] -= 1;
654654
if (this.subscribedClusters[clusterName] <= 0) {
655655
delete this.subscribedClusters[clusterName];
656-
this.pruneOrphanClusters();
657-
this.maybeSendUpdate();
656+
if (this.pruneOrphanClusters()) {
657+
this.maybeSendUpdate();
658+
}
658659
}
659660
}
660661
}
@@ -682,7 +683,12 @@ export class XdsDependencyManager {
682683
delete this.clusterForest[clusterName];
683684
}
684685

685-
private pruneOrphanClusters() {
686+
/**
687+
* Prune any clusters that are not descendents of any root clusters,
688+
* including subscribed clusters.
689+
* @returns True if any clusters were pruned, false otherwise
690+
*/
691+
private pruneOrphanClusters(): boolean {
686692
const toCheck = [...this.clusterRoots, ...Object.keys(this.subscribedClusters)];
687693
const visited = new Set<string>();
688694
while(toCheck.length > 0) {
@@ -695,11 +701,14 @@ export class XdsDependencyManager {
695701
}
696702
visited.add(next);
697703
}
704+
let removedAnyClusters = false;
698705
for (const clusterName of Object.keys(this.clusterForest)) {
699706
if (!visited.has(clusterName)) {
707+
removedAnyClusters = true;
700708
this.removeCluster(clusterName);
701709
}
702710
}
711+
return removedAnyClusters;
703712
}
704713

705714
private handleRouteConfig(routeConfig: RouteConfiguration__Output) {

packages/grpc-js/src/backoff-timeout.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export class BackoffTimeout {
107107
private runTimer(delay: number) {
108108
this.endTime = this.startTime;
109109
this.endTime.setMilliseconds(
110-
this.endTime.getMilliseconds() + this.nextDelay
110+
this.endTime.getMilliseconds() + delay
111111
);
112112
clearTimeout(this.timerId);
113113
this.timerId = setTimeout(() => {

packages/grpc-js/src/load-balancer-pick-first.ts

+7
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,13 @@ export class PickFirstLoadBalancer implements LoadBalancer {
275275
new PickFirstPicker(this.currentPick)
276276
);
277277
}
278+
} else if (this.latestAddressList?.length === 0) {
279+
this.updateState(
280+
ConnectivityState.TRANSIENT_FAILURE,
281+
new UnavailablePicker({
282+
details: `No connection established. Last error: ${this.lastError}`,
283+
})
284+
);
278285
} else if (this.children.length === 0) {
279286
this.updateState(ConnectivityState.IDLE, new QueuePicker(this));
280287
} else {

packages/grpc-js/test/test-pick-first.ts

+13
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,19 @@ describe('pick_first load balancing policy', () => {
690690
});
691691
});
692692
});
693+
it('Should report TRANSIENT_FAILURE with no addresses', done => {
694+
const channelControlHelper = createChildChannelControlHelper(
695+
baseChannelControlHelper,
696+
{
697+
updateState: updateStateCallBackForExpectedStateSequence(
698+
[ConnectivityState.TRANSIENT_FAILURE],
699+
done
700+
),
701+
}
702+
);
703+
const pickFirst = new PickFirstLoadBalancer(channelControlHelper, creds, {});
704+
pickFirst.updateAddressList([], config);
705+
});
693706
describe('Address list randomization', () => {
694707
const shuffleConfig = new PickFirstLoadBalancingConfig(true);
695708
it('Should pick different subchannels after multiple updates', done => {

0 commit comments

Comments
 (0)