Skip to content

Commit

Permalink
Merge pull request #2181 from murgatroid99/grpc-js_outlier_detection_…
Browse files Browse the repository at this point in the history
…fixes_backport

grpc-js: backport outlier detection fixes to v1.6.x
  • Loading branch information
murgatroid99 committed Aug 8, 2022
2 parents b08171e + 36f37cb commit a2e5ded
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 10 deletions.
2 changes: 1 addition & 1 deletion packages/grpc-js-xds/interop/Dockerfile
Expand Up @@ -33,6 +33,6 @@ COPY --from=build /node/src/grpc-node/packages/grpc-js ./packages/grpc-js/
COPY --from=build /node/src/grpc-node/packages/grpc-js-xds ./packages/grpc-js-xds/

ENV GRPC_VERBOSITY="DEBUG"
ENV GRPC_TRACE=xds_client,xds_resolver,cds_balancer,eds_balancer,priority,weighted_target,round_robin,resolving_load_balancer,subchannel,keepalive,dns_resolver,fault_injection,http_filter,csds
ENV GRPC_TRACE=xds_client,xds_resolver,cds_balancer,eds_balancer,priority,weighted_target,round_robin,resolving_load_balancer,subchannel,keepalive,dns_resolver,fault_injection,http_filter,csds,outlier_detection

ENTRYPOINT [ "node", "/node/src/grpc-node/packages/grpc-js-xds/build/interop/xds-interop-client" ]
5 changes: 2 additions & 3 deletions packages/grpc-js-xds/src/load-balancer-cds.ts
Expand Up @@ -79,9 +79,8 @@ function translateOutlierDetectionConfig(outlierDetection: OutlierDetection__Out
return undefined;
}
if (!outlierDetection) {
/* No-op outlier detection config, with max possible interval and no
* ejection criteria configured. */
return new OutlierDetectionLoadBalancingConfig(~(1<<31), null, null, null, null, null, []);
/* No-op outlier detection config, with all fields unset. */
return new OutlierDetectionLoadBalancingConfig(null, null, null, null, null, null, []);
}
let successRateConfig: Partial<SuccessRateEjectionConfig> | null = null;
/* Success rate ejection is enabled by default, so we only disable it if
Expand Down
55 changes: 49 additions & 6 deletions packages/grpc-js/src/load-balancer-outlier-detection.ts
Expand Up @@ -18,7 +18,7 @@
import { ChannelOptions, connectivityState, StatusObject } from ".";
import { Call } from "./call-stream";
import { ConnectivityState } from "./connectivity-state";
import { Status } from "./constants";
import { LogVerbosity, Status } from "./constants";
import { durationToMs, isDuration, msToDuration } from "./duration";
import { ChannelControlHelper, createChildChannelControlHelper, registerLoadBalancerType } from "./experimental";
import { BaseFilter, Filter, FilterFactory } from "./filter";
Expand All @@ -28,7 +28,13 @@ import { PickArgs, Picker, PickResult, PickResultType, QueuePicker, UnavailableP
import { Subchannel } from "./subchannel";
import { SubchannelAddress, subchannelAddressToString } from "./subchannel-address";
import { BaseSubchannelWrapper, ConnectivityStateListener, SubchannelInterface } from "./subchannel-interface";
import * as logging from './logging';

const TRACER_NAME = 'outlier_detection';

function trace(text: string): void {
logging.trace(LogVerbosity.DEBUG, TRACER_NAME, text);
}

const TYPE_NAME = 'outlier_detection';

Expand Down Expand Up @@ -193,12 +199,13 @@ export class OutlierDetectionLoadBalancingConfig implements LoadBalancingConfig
}

class OutlierDetectionSubchannelWrapper extends BaseSubchannelWrapper implements SubchannelInterface {
private childSubchannelState: ConnectivityState = ConnectivityState.IDLE;
private childSubchannelState: ConnectivityState;
private stateListeners: ConnectivityStateListener[] = [];
private ejected: boolean = false;
private refCount: number = 0;
constructor(childSubchannel: SubchannelInterface, private mapEntry?: MapEntry) {
super(childSubchannel);
this.childSubchannelState = childSubchannel.getConnectivityState();
childSubchannel.addConnectivityStateListener((subchannel, previousState, newState) => {
this.childSubchannelState = newState;
if (!this.ejected) {
Expand All @@ -209,6 +216,14 @@ class OutlierDetectionSubchannelWrapper extends BaseSubchannelWrapper implements
});
}

getConnectivityState(): connectivityState {
if (this.ejected) {
return ConnectivityState.TRANSIENT_FAILURE;
} else {
return this.childSubchannelState;
}
}

/**
* Add a listener function to be called whenever the wrapper's
* connectivity state changes.
Expand Down Expand Up @@ -351,7 +366,10 @@ class OutlierDetectionPicker implements Picker {
extraFilterFactories: extraFilterFactories
};
} else {
return wrappedPick;
return {
...wrappedPick,
subchannel: subchannelWrapper.getWrappedSubchannel()
}
}
} else {
return wrappedPick;
Expand All @@ -373,6 +391,10 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
const originalSubchannel = channelControlHelper.createSubchannel(subchannelAddress, subchannelArgs);
const mapEntry = this.addressMap.get(subchannelAddressToString(subchannelAddress));
const subchannelWrapper = new OutlierDetectionSubchannelWrapper(originalSubchannel, mapEntry);
if (mapEntry?.currentEjectionTimestamp !== null) {
// If the address is ejected, propagate that to the new subchannel wrapper
subchannelWrapper.eject();
}
mapEntry?.subchannelWrappers.push(subchannelWrapper);
return subchannelWrapper;
},
Expand Down Expand Up @@ -412,6 +434,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
if (!successRateConfig) {
return;
}
trace('Running success rate check');
// Step 1
const targetRequestVolume = successRateConfig.request_volume;
let addresesWithTargetVolume = 0;
Expand All @@ -424,6 +447,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
successRates.push(successes/(successes + failures));
}
}
trace('Found ' + addresesWithTargetVolume + ' success rate candidates; currentEjectionPercent=' + this.getCurrentEjectionPercent() + ' successRates=[' + successRates + ']');
if (addresesWithTargetVolume < successRateConfig.minimum_hosts) {
return;
}
Expand All @@ -438,9 +462,10 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
const successRateVariance = successRateDeviationSum / successRates.length;
const successRateStdev = Math.sqrt(successRateVariance);
const ejectionThreshold = successRateMean - successRateStdev * (successRateConfig.stdev_factor / 1000);
trace('stdev=' + successRateStdev + ' ejectionThreshold=' + ejectionThreshold);

// Step 3
for (const mapEntry of this.addressMap.values()) {
for (const [address, mapEntry] of this.addressMap.entries()) {
// Step 3.i
if (this.getCurrentEjectionPercent() > this.latestConfig.getMaxEjectionPercent()) {
break;
Expand All @@ -453,9 +478,12 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
}
// Step 3.iii
const successRate = successes / (successes + failures);
trace('Checking candidate ' + address + ' successRate=' + successRate);
if (successRate < ejectionThreshold) {
const randomNumber = Math.random() * 100;
trace('Candidate ' + address + ' randomNumber=' + randomNumber + ' enforcement_percentage=' + successRateConfig.enforcement_percentage);
if (randomNumber < successRateConfig.enforcement_percentage) {
trace('Ejecting candidate ' + address);
this.eject(mapEntry, ejectionTimestamp);
}
}
Expand All @@ -470,28 +498,32 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
if (!failurePercentageConfig) {
return;
}
trace('Running failure percentage check. threshold=' + failurePercentageConfig.threshold + ' request volume threshold=' + failurePercentageConfig.request_volume);
// Step 1
if (this.addressMap.size < failurePercentageConfig.minimum_hosts) {
return;
}

// Step 2
for (const mapEntry of this.addressMap.values()) {
for (const [address, mapEntry] of this.addressMap.entries()) {
// Step 2.i
if (this.getCurrentEjectionPercent() > this.latestConfig.getMaxEjectionPercent()) {
break;
}
// Step 2.ii
const successes = mapEntry.counter.getLastSuccesses();
const failures = mapEntry.counter.getLastFailures();
trace('Candidate successes=' + successes + ' failures=' + failures);
if (successes + failures < failurePercentageConfig.request_volume) {
continue;
}
// Step 2.iii
const failurePercentage = (failures * 100) / (failures + successes);
if (failurePercentage > failurePercentageConfig.threshold) {
const randomNumber = Math.random() * 100;
trace('Candidate ' + address + ' randomNumber=' + randomNumber + ' enforcement_percentage=' + failurePercentageConfig.enforcement_percentage);
if (randomNumber < failurePercentageConfig.enforcement_percentage) {
trace('Ejecting candidate ' + address);
this.eject(mapEntry, ejectionTimestamp);
}
}
Expand Down Expand Up @@ -525,6 +557,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {

private runChecks() {
const ejectionTimestamp = new Date();
trace('Ejection timer running');

this.switchAllBuckets();

Expand All @@ -537,7 +570,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
this.runSuccessRateCheck(ejectionTimestamp);
this.runFailurePercentageCheck(ejectionTimestamp);

for (const mapEntry of this.addressMap.values()) {
for (const [address, mapEntry] of this.addressMap.entries()) {
if (mapEntry.currentEjectionTimestamp === null) {
if (mapEntry.ejectionTimeMultiplier > 0) {
mapEntry.ejectionTimeMultiplier -= 1;
Expand All @@ -548,6 +581,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
const returnTime = new Date(mapEntry.currentEjectionTimestamp.getTime());
returnTime.setMilliseconds(returnTime.getMilliseconds() + Math.min(baseEjectionTimeMs * mapEntry.ejectionTimeMultiplier, Math.max(baseEjectionTimeMs, maxEjectionTimeMs)));
if (returnTime < new Date()) {
trace('Unejecting ' + address);
this.uneject(mapEntry);
}
}
Expand All @@ -564,6 +598,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
}
for (const address of subchannelAddresses) {
if (!this.addressMap.has(address)) {
trace('Adding map entry for ' + address);
this.addressMap.set(address, {
counter: new CallCounter(),
currentEjectionTimestamp: null,
Expand All @@ -574,6 +609,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {
}
for (const key of this.addressMap.keys()) {
if (!subchannelAddresses.has(key)) {
trace('Removing map entry for ' + key);
this.addressMap.delete(key);
}
}
Expand All @@ -585,17 +621,24 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer {

if (lbConfig.getSuccessRateEjectionConfig() || lbConfig.getFailurePercentageEjectionConfig()) {
if (this.timerStartTime) {
trace('Previous timer existed. Replacing timer');
clearTimeout(this.ejectionTimer);
const remainingDelay = lbConfig.getIntervalMs() - ((new Date()).getTime() - this.timerStartTime.getTime());
this.startTimer(remainingDelay);
} else {
trace('Starting new timer');
this.timerStartTime = new Date();
this.startTimer(lbConfig.getIntervalMs());
this.switchAllBuckets();
}
} else {
trace('Counting disabled. Cancelling timer.');
this.timerStartTime = null;
clearTimeout(this.ejectionTimer);
for (const mapEntry of this.addressMap.values()) {
this.uneject(mapEntry);
mapEntry.ejectionTimeMultiplier = 0;
}
}

this.latestConfig = lbConfig;
Expand Down

0 comments on commit a2e5ded

Please sign in to comment.