From bb6bb4a5ca79de4946c309f30cfffe0ebd893021 Mon Sep 17 00:00:00 2001
From: Michael Lumish <mlumish@google.com>
Date: Tue, 7 Jan 2025 14:24:25 -0800
Subject: [PATCH 1/6] grpc-js-xds: Preserve resource type version and nonce
when unsubscribing
---
packages/grpc-js-xds/src/xds-client.ts | 3 ---
1 file changed, 3 deletions(-)
diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts
index 0879d6b27..eb37da978 100644
--- a/packages/grpc-js-xds/src/xds-client.ts
+++ b/packages/grpc-js-xds/src/xds-client.ts
@@ -457,9 +457,6 @@ class AdsCallState {
if (authorityMap.size === 0) {
typeState.subscribedResources.delete(name.authority);
}
- if (typeState.subscribedResources.size === 0) {
- this.typeStates.delete(type);
- }
this.updateNames(type);
}
From b396b7d5ae2792fd34eefacf29d615b32cdfab5d Mon Sep 17 00:00:00 2001
From: Michael Lumish <mlumish@google.com>
Date: Wed, 8 Jan 2025 14:56:38 -0800
Subject: [PATCH 2/6] grpc-js-xds: Bump to version 1.12.1
---
packages/grpc-js-xds/package.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/packages/grpc-js-xds/package.json b/packages/grpc-js-xds/package.json
index f6cb72352..09fd3cdb5 100644
--- a/packages/grpc-js-xds/package.json
+++ b/packages/grpc-js-xds/package.json
@@ -1,6 +1,6 @@
{
"name": "@grpc/grpc-js-xds",
- "version": "1.12.0",
+ "version": "1.12.1",
"description": "Plugin for @grpc/grpc-js. Adds the xds:// URL scheme and associated features.",
"main": "build/src/index.js",
"scripts": {
From ca21e4ab1f3bc6938c4fbac7e00c428e18e11165 Mon Sep 17 00:00:00 2001
From: Michael Lumish <mlumish@google.com>
Date: Fri, 31 Jan 2025 11:24:19 -0800
Subject: [PATCH 3/6] grpc-js: Allow garbage collection of IDLE channels
---
packages/grpc-js/package.json | 2 +-
packages/grpc-js/src/internal-channel.ts | 96 ++++++++++++++----------
2 files changed, 59 insertions(+), 39 deletions(-)
diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json
index b11495c6f..b54b1a8da 100644
--- a/packages/grpc-js/package.json
+++ b/packages/grpc-js/package.json
@@ -1,6 +1,6 @@
{
"name": "@grpc/grpc-js",
- "version": "1.12.5",
+ "version": "1.12.6",
"description": "gRPC Library for Node - pure JS implementation",
"homepage": "https://grpc.io/",
"repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",
diff --git a/packages/grpc-js/src/internal-channel.ts b/packages/grpc-js/src/internal-channel.ts
index 874729790..d53a9dc26 100644
--- a/packages/grpc-js/src/internal-channel.ts
+++ b/packages/grpc-js/src/internal-channel.ts
@@ -125,10 +125,13 @@ class ChannelSubchannelWrapper
) => {
channel.throttleKeepalive(keepaliveTime);
};
- childSubchannel.addConnectivityStateListener(this.subchannelStateListener);
}
ref(): void {
+ if (this.refCount === 0) {
+ this.child.addConnectivityStateListener(this.subchannelStateListener);
+ this.channel.addWrappedSubchannel(this);
+ }
this.child.ref();
this.refCount += 1;
}
@@ -159,6 +162,26 @@ class ShutdownPicker implements Picker {
}
}
+class ChannelzInfoTracker {
+ readonly trace = new ChannelzTrace();
+ readonly callTracker = new ChannelzCallTracker();
+ readonly childrenTracker = new ChannelzChildrenTracker();
+ state: ConnectivityState = ConnectivityState.IDLE;
+ constructor(private target: string) {}
+
+ getChannelzInfoCallback(): () => ChannelInfo {
+ return () => {
+ return {
+ target: this.target,
+ state: this.state,
+ trace: this.trace,
+ callTracker: this.callTracker,
+ children: this.childrenTracker.getChildLists()
+ };
+ };
+ }
+}
+
export class InternalChannel {
private readonly resolvingLoadBalancer: ResolvingLoadBalancer;
private readonly subchannelPool: SubchannelPool;
@@ -179,9 +202,10 @@ export class InternalChannel {
* event loop open while there are any pending calls for the channel that
* have not yet been assigned to specific subchannels. In other words,
* the invariant is that callRefTimer is reffed if and only if pickQueue
- * is non-empty.
+ * is non-empty. In addition, the timer is null while the state is IDLE or
+ * SHUTDOWN and there are no pending calls.
*/
- private readonly callRefTimer: NodeJS.Timeout;
+ private callRefTimer: NodeJS.Timeout | null = null;
private configSelector: ConfigSelector | null = null;
/**
* This is the error from the name resolver if it failed most recently. It
@@ -203,11 +227,8 @@ export class InternalChannel {
// Channelz info
private readonly channelzEnabled: boolean = true;
- private readonly originalTarget: string;
private readonly channelzRef: ChannelRef;
- private readonly channelzTrace: ChannelzTrace;
- private readonly callTracker = new ChannelzCallTracker();
- private readonly childrenTracker = new ChannelzChildrenTracker();
+ private readonly channelzInfoTracker: ChannelzInfoTracker;
/**
* Randomly generated ID to be passed to the config selector, for use by
@@ -236,7 +257,7 @@ export class InternalChannel {
throw new TypeError('Channel options must be an object');
}
}
- this.originalTarget = target;
+ this.channelzInfoTracker = new ChannelzInfoTracker(target);
const originalTargetUri = parseUri(target);
if (originalTargetUri === null) {
throw new Error(`Could not parse target name "${target}"`);
@@ -250,21 +271,17 @@ export class InternalChannel {
);
}
- this.callRefTimer = setInterval(() => {}, MAX_TIMEOUT_TIME);
- this.callRefTimer.unref?.();
-
if (this.options['grpc.enable_channelz'] === 0) {
this.channelzEnabled = false;
}
- this.channelzTrace = new ChannelzTrace();
this.channelzRef = registerChannelzChannel(
target,
- () => this.getChannelzInfo(),
+ this.channelzInfoTracker.getChannelzInfoCallback(),
this.channelzEnabled
);
if (this.channelzEnabled) {
- this.channelzTrace.addTrace('CT_INFO', 'Channel created');
+ this.channelzInfoTracker.trace.addTrace('CT_INFO', 'Channel created');
}
if (this.options['grpc.default_authority']) {
@@ -305,7 +322,7 @@ export class InternalChannel {
);
subchannel.throttleKeepalive(this.keepaliveTime);
if (this.channelzEnabled) {
- this.channelzTrace.addTrace(
+ this.channelzInfoTracker.trace.addTrace(
'CT_INFO',
'Created subchannel or used existing subchannel',
subchannel.getChannelzRef()
@@ -315,7 +332,6 @@ export class InternalChannel {
subchannel,
this
);
- this.wrappedSubchannels.add(wrappedSubchannel);
return wrappedSubchannel;
},
updateState: (connectivityState: ConnectivityState, picker: Picker) => {
@@ -338,12 +354,12 @@ export class InternalChannel {
},
addChannelzChild: (child: ChannelRef | SubchannelRef) => {
if (this.channelzEnabled) {
- this.childrenTracker.refChild(child);
+ this.channelzInfoTracker.childrenTracker.refChild(child);
}
},
removeChannelzChild: (child: ChannelRef | SubchannelRef) => {
if (this.channelzEnabled) {
- this.childrenTracker.unrefChild(child);
+ this.channelzInfoTracker.childrenTracker.unrefChild(child);
}
},
};
@@ -366,7 +382,7 @@ export class InternalChannel {
RETRY_THROTTLER_MAP.delete(this.getTarget());
}
if (this.channelzEnabled) {
- this.channelzTrace.addTrace(
+ this.channelzInfoTracker.trace.addTrace(
'CT_INFO',
'Address resolution succeeded'
);
@@ -388,7 +404,7 @@ export class InternalChannel {
},
status => {
if (this.channelzEnabled) {
- this.channelzTrace.addTrace(
+ this.channelzInfoTracker.trace.addTrace(
'CT_WARNING',
'Address resolution failed with code ' +
status.code +
@@ -440,16 +456,6 @@ export class InternalChannel {
this.lastActivityTimestamp = new Date();
}
- private getChannelzInfo(): ChannelInfo {
- return {
- target: this.originalTarget,
- state: this.connectivityState,
- trace: this.channelzTrace,
- callTracker: this.callTracker,
- children: this.childrenTracker.getChildLists(),
- };
- }
-
private trace(text: string, verbosityOverride?: LogVerbosity) {
trace(
verbosityOverride ?? LogVerbosity.DEBUG,
@@ -459,6 +465,9 @@ export class InternalChannel {
}
private callRefTimerRef() {
+ if (!this.callRefTimer) {
+ this.callRefTimer = setInterval(() => {}, MAX_TIMEOUT_TIME)
+ }
// If the hasRef function does not exist, always run the code
if (!this.callRefTimer.hasRef?.()) {
this.trace(
@@ -472,15 +481,15 @@ export class InternalChannel {
}
private callRefTimerUnref() {
- // If the hasRef function does not exist, always run the code
- if (!this.callRefTimer.hasRef || this.callRefTimer.hasRef()) {
+ // If the timer or the hasRef function does not exist, always run the code
+ if (!this.callRefTimer?.hasRef || this.callRefTimer.hasRef()) {
this.trace(
'callRefTimer.unref | configSelectionQueue.length=' +
this.configSelectionQueue.length +
' pickQueue.length=' +
this.pickQueue.length
);
- this.callRefTimer.unref?.();
+ this.callRefTimer?.unref?.();
}
}
@@ -509,12 +518,13 @@ export class InternalChannel {
ConnectivityState[newState]
);
if (this.channelzEnabled) {
- this.channelzTrace.addTrace(
+ this.channelzInfoTracker.trace.addTrace(
'CT_INFO',
'Connectivity state change to ' + ConnectivityState[newState]
);
}
this.connectivityState = newState;
+ this.channelzInfoTracker.state = newState;
const watchersCopy = this.connectivityStateWatchers.slice();
for (const watcherObject of watchersCopy) {
if (newState !== watcherObject.currentState) {
@@ -539,6 +549,10 @@ export class InternalChannel {
}
}
+ addWrappedSubchannel(wrappedSubchannel: ChannelSubchannelWrapper) {
+ this.wrappedSubchannels.add(wrappedSubchannel);
+ }
+
removeWrappedSubchannel(wrappedSubchannel: ChannelSubchannelWrapper) {
this.wrappedSubchannels.delete(wrappedSubchannel);
}
@@ -591,6 +605,10 @@ export class InternalChannel {
clearTimeout(this.idleTimer);
this.idleTimer = null;
}
+ if (this.callRefTimer) {
+ clearInterval(this.callRefTimer);
+ this.callRefTimer = null;
+ }
}
private startIdleTimeout(timeoutMs: number) {
@@ -634,7 +652,7 @@ export class InternalChannel {
private onCallStart() {
if (this.channelzEnabled) {
- this.callTracker.addCallStarted();
+ this.channelzInfoTracker.callTracker.addCallStarted();
}
this.callCount += 1;
}
@@ -642,9 +660,9 @@ export class InternalChannel {
private onCallEnd(status: StatusObject) {
if (this.channelzEnabled) {
if (status.code === Status.OK) {
- this.callTracker.addCallSucceeded();
+ this.channelzInfoTracker.callTracker.addCallSucceeded();
} else {
- this.callTracker.addCallFailed();
+ this.channelzInfoTracker.callTracker.addCallFailed();
}
}
this.callCount -= 1;
@@ -776,7 +794,9 @@ export class InternalChannel {
call.cancelWithStatus(Status.UNAVAILABLE, 'Channel closed before call started');
}
this.pickQueue = [];
- clearInterval(this.callRefTimer);
+ if (this.callRefTimer) {
+ clearInterval(this.callRefTimer);
+ }
if (this.idleTimer) {
clearTimeout(this.idleTimer);
}
From 621f401e348776c411b101e768ced63ed017055e Mon Sep 17 00:00:00 2001
From: gfrancz <52467377+gfrancz@users.noreply.github.com>
Date: Mon, 27 Jan 2025 21:46:34 -0500
Subject: [PATCH 4/6] fix: xds-client LrsCallState statsTimer memory leak
---
packages/grpc-js-xds/src/xds-client.ts | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts
index eb37da978..71b03ab5e 100644
--- a/packages/grpc-js-xds/src/xds-client.ts
+++ b/packages/grpc-js-xds/src/xds-client.ts
@@ -682,6 +682,13 @@ class LrsCallState {
this.sendStats();
}
+ destroy() {
+ if (this.statsTimer) {
+ this.statsTimer = clearInterval(this.statsTimer);
+ }
+ return null;
+ }
+
private handleStreamStatus(status: StatusObject) {
this.client.trace(
'LRS stream ended. code=' + status.code + ' details= ' + status.details
@@ -932,7 +939,7 @@ class XdsSingleServerClient {
}
handleLrsStreamEnd() {
- this.lrsCallState = null;
+ this.lrsCallState = this.lrsCallState ? this.lrsCallState.destroy() : null;
/* The backoff timer would start the stream when it finishes. If it is not
* running, restart the stream immediately. */
if (!this.lrsBackoff.isRunning()) {
From 588b69c12c884991c415665d2e37492464ba6eb8 Mon Sep 17 00:00:00 2001
From: gfrancz <52467377+gfrancz@users.noreply.github.com>
Date: Tue, 4 Feb 2025 13:55:16 -0500
Subject: [PATCH 5/6] Fix type issue by setting the attribute separately
Co-authored-by: ws-gregm <55088361+ws-gregm@users.noreply.github.com>
---
packages/grpc-js-xds/src/xds-client.ts | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts
index 71b03ab5e..b4d6e24fc 100644
--- a/packages/grpc-js-xds/src/xds-client.ts
+++ b/packages/grpc-js-xds/src/xds-client.ts
@@ -684,7 +684,8 @@ class LrsCallState {
destroy() {
if (this.statsTimer) {
- this.statsTimer = clearInterval(this.statsTimer);
+ clearInterval(this.statsTimer);
+ this.statsTimer = null;
}
return null;
}
From f1f2b2dd83cf0c3e92e391db85e05b3a9761436c Mon Sep 17 00:00:00 2001
From: Michael Lumish <mlumish@google.com>
Date: Tue, 4 Feb 2025 14:14:37 -0800
Subject: [PATCH 6/6] grpc-js-xds: Bump to version 1.12.2
---
packages/grpc-js-xds/package.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/packages/grpc-js-xds/package.json b/packages/grpc-js-xds/package.json
index 09fd3cdb5..512ad77ed 100644
--- a/packages/grpc-js-xds/package.json
+++ b/packages/grpc-js-xds/package.json
@@ -1,6 +1,6 @@
{
"name": "@grpc/grpc-js-xds",
- "version": "1.12.1",
+ "version": "1.12.2",
"description": "Plugin for @grpc/grpc-js. Adds the xds:// URL scheme and associated features.",
"main": "build/src/index.js",
"scripts": {