Skip to content

Commit

Permalink
Merge pull request #2806 from murgatroid99/grpc-js_connectivity_manag…
Browse files Browse the repository at this point in the history 
…ement_fixes

grpc-js: Simplify pick_first behavior
  • Loading branch information
@murgatroid99
murgatroid99 authored Sep 16, 2024
2 parents 0c5ab98 + 3d43b1a commit a1b8972
Showing 1 changed file with 33 additions and 62 deletions.
95 changes: 33 additions & 62 deletions packages/grpc-js/src/load-balancer-pick-first.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,6 @@ export class PickFirstLoadBalancer implements LoadBalancer {
*/
private connectionDelayTimeout: NodeJS.Timeout;

private triedAllSubchannels = false;

/**
* The LB policy enters sticky TRANSIENT_FAILURE mode when all
* subchannels have failed to connect at least once, and it stays in that
Expand All @@ -225,12 +223,6 @@ export class PickFirstLoadBalancer implements LoadBalancer {

private reportHealthStatus: boolean;

/**
* Indicates whether we called channelControlHelper.requestReresolution since
* the last call to updateAddressList
*/
private requestedResolutionSinceLastUpdate = false;

/**
* The most recent error reported by any subchannel as it transitioned to
* TRANSIENT_FAILURE.
Expand Down Expand Up @@ -259,6 +251,10 @@ export class PickFirstLoadBalancer implements LoadBalancer {
return this.children.every(child => child.hasReportedTransientFailure);
}

private resetChildrenReportedTF() {
this.children.every(child => child.hasReportedTransientFailure = false);
}

private calculateAndReportNewState() {
if (this.currentPick) {
if (this.reportHealthStatus && !this.currentPick.isHealthy()) {
Expand Down Expand Up @@ -291,23 +287,15 @@ export class PickFirstLoadBalancer implements LoadBalancer {
}

private requestReresolution() {
this.requestedResolutionSinceLastUpdate = true;
this.channelControlHelper.requestReresolution();
}

private maybeEnterStickyTransientFailureMode() {
if (!this.allChildrenHaveReportedTF()) {
return;
}
if (!this.requestedResolutionSinceLastUpdate) {
/* Each time we get an update we reset each subchannel's
* hasReportedTransientFailure flag, so the next time we get to this
* point after that, each subchannel has reported TRANSIENT_FAILURE
* at least once since then. That is the trigger for requesting
* reresolution, whether or not the LB policy is already in sticky TF
* mode. */
this.requestReresolution();
}
this.requestReresolution();
this.resetChildrenReportedTF();
if (this.stickyTransientFailureMode) {
return;
}
Expand All @@ -320,21 +308,16 @@ export class PickFirstLoadBalancer implements LoadBalancer {

private removeCurrentPick() {
if (this.currentPick !== null) {
/* Unref can cause a state change, which can cause a change in the value
* of this.currentPick, so we hold a local reference to make sure that
* does not impact this function. */
const currentPick = this.currentPick;
this.currentPick = null;
currentPick.unref();
currentPick.removeConnectivityStateListener(this.subchannelStateListener);
this.currentPick.removeConnectivityStateListener(this.subchannelStateListener);
this.channelControlHelper.removeChannelzChild(
currentPick.getChannelzRef()
this.currentPick.getChannelzRef()
);
if (this.reportHealthStatus) {
currentPick.removeHealthStateWatcher(
this.pickedSubchannelHealthListener
);
}
this.currentPick.removeHealthStateWatcher(
this.pickedSubchannelHealthListener
);
// Unref last, to avoid triggering listeners
this.currentPick.unref();
this.currentPick = null;
}
}

Expand Down Expand Up @@ -374,9 +357,6 @@ export class PickFirstLoadBalancer implements LoadBalancer {

private startNextSubchannelConnecting(startIndex: number) {
clearTimeout(this.connectionDelayTimeout);
if (this.triedAllSubchannels) {
return;
}
for (const [index, child] of this.children.entries()) {
if (index >= startIndex) {
const subchannelState = child.subchannel.getConnectivityState();
Expand All @@ -389,7 +369,6 @@ export class PickFirstLoadBalancer implements LoadBalancer {
}
}
}
this.triedAllSubchannels = true;
this.maybeEnterStickyTransientFailureMode();
}

Expand Down Expand Up @@ -418,20 +397,25 @@ export class PickFirstLoadBalancer implements LoadBalancer {
this.connectionDelayTimeout.unref?.();
}

/**
* Declare that the specified subchannel should be used to make requests.
* This functions the same independent of whether subchannel is a member of
* this.children and whether it is equal to this.currentPick.
* Prerequisite: subchannel.getConnectivityState() === READY.
* @param subchannel
*/
private pickSubchannel(subchannel: SubchannelInterface) {
if (this.currentPick && subchannel.realSubchannelEquals(this.currentPick)) {
return;
}
trace('Pick subchannel with address ' + subchannel.getAddress());
this.stickyTransientFailureMode = false;
this.removeCurrentPick();
this.currentPick = subchannel;
/* Ref before removeCurrentPick and resetSubchannelList to avoid the
* refcount dropping to 0 during this process. */
subchannel.ref();
if (this.reportHealthStatus) {
subchannel.addHealthStateWatcher(this.pickedSubchannelHealthListener);
}
this.channelControlHelper.addChannelzChild(subchannel.getChannelzRef());
this.removeCurrentPick();
this.resetSubchannelList();
subchannel.addConnectivityStateListener(this.subchannelStateListener);
subchannel.addHealthStateWatcher(this.pickedSubchannelHealthListener);
this.currentPick = subchannel;
clearTimeout(this.connectionDelayTimeout);
this.calculateAndReportNewState();
}
Expand All @@ -448,20 +432,11 @@ export class PickFirstLoadBalancer implements LoadBalancer {

private resetSubchannelList() {
for (const child of this.children) {
if (
!(
this.currentPick &&
child.subchannel.realSubchannelEquals(this.currentPick)
)
) {
/* The connectivity state listener is the same whether the subchannel
* is in the list of children or it is the currentPick, so if it is in
* both, removing it here would cause problems. In particular, that
* always happens immediately after the subchannel is picked. */
child.subchannel.removeConnectivityStateListener(
this.subchannelStateListener
);
}
/* Always remoev the connectivity state listener. If the subchannel is
getting picked, it will be re-added then. */
child.subchannel.removeConnectivityStateListener(
this.subchannelStateListener
);
/* Refs are counted independently for the children list and the
* currentPick, so we call unref whether or not the child is the
* currentPick. Channelz child references are also refcounted, so
Expand All @@ -473,20 +448,16 @@ export class PickFirstLoadBalancer implements LoadBalancer {
}
this.currentSubchannelIndex = 0;
this.children = [];
this.triedAllSubchannels = false;
this.requestedResolutionSinceLastUpdate = false;
}

private connectToAddressList(addressList: SubchannelAddress[]) {
trace('connectToAddressList([' + addressList.map(address => subchannelAddressToString(address)) + '])');
const newChildrenList = addressList.map(address => ({
subchannel: this.channelControlHelper.createSubchannel(address, {}),
hasReportedTransientFailure: false,
}));
trace('connectToAddressList([' + addressList.map(address => subchannelAddressToString(address)) + '])');
for (const { subchannel } of newChildrenList) {
if (subchannel.getConnectivityState() === ConnectivityState.READY) {
this.channelControlHelper.addChannelzChild(subchannel.getChannelzRef());
subchannel.addConnectivityStateListener(this.subchannelStateListener);
this.pickSubchannel(subchannel);
return;
}
Expand Down

0 comments on commit a1b8972

Please sign in to comment.