rusackas commented on code in PR #40856:
URL: https://github.com/apache/superset/pull/40856#discussion_r3375295563
##########
superset-websocket/src/index.ts:
##########
@@ -159,6 +159,37 @@ export const setLastFirehoseId = (id: string): void => {
lastFirehoseId = id;
};
+// WebSocket close code used when a connection is refused because a configured
+// connection limit has been reached (1013 = "Try Again Later").
+const CONNECTION_LIMIT_CLOSE_CODE = 1013;
+
+/**
+ * Determines whether accepting a new connection on the given channel would
+ * exceed a configured connection limit. Returns a human-readable reason when a
+ * limit is reached, or `null` when the connection is within limits.
+ *
+ * Both limits are opt-in: a value of `0` (the default) disables the check.
+ */
+export const connectionLimitReason = (channel: string): string | null => {
+ const { maxTotalConnections, maxConnectionsPerChannel } = opts;
+
+ if (
+ maxTotalConnections > 0 &&
+ Object.keys(sockets).length >= maxTotalConnections
+ ) {
+ return `total connection limit (${maxTotalConnections}) reached`;
+ }
+
+ if (maxConnectionsPerChannel > 0) {
+ const channelSize = channels[channel]?.sockets.length ?? 0;
+ if (channelSize >= maxConnectionsPerChannel) {
Review Comment:
Good catch. Fixed in 163acf7b81: the limit checks now count only sockets
whose underlying connection is in an active readyState (SOCKET_ACTIVE_STATES)
rather than the raw registry sizes, via small
`isSocketActive`/`activeSocketCount`/`activeChannelSocketCount` helpers.
Sockets that have closed but not yet been GC'd by `checkSockets`/`cleanChannel`
no longer count toward the total or per-channel caps. Added tests covering the
stale-closed-socket case for both limits.
##########
superset-websocket/src/index.ts:
##########
@@ -159,6 +159,37 @@ export const setLastFirehoseId = (id: string): void => {
lastFirehoseId = id;
};
+// WebSocket close code used when a connection is refused because a configured
+// connection limit has been reached (1013 = "Try Again Later").
+const CONNECTION_LIMIT_CLOSE_CODE = 1013;
+
+/**
+ * Determines whether accepting a new connection on the given channel would
+ * exceed a configured connection limit. Returns a human-readable reason when a
+ * limit is reached, or `null` when the connection is within limits.
+ *
+ * Both limits are opt-in: a value of `0` (the default) disables the check.
+ */
+export const connectionLimitReason = (channel: string): string | null => {
+ const { maxTotalConnections, maxConnectionsPerChannel } = opts;
+
+ if (
+ maxTotalConnections > 0 &&
+ Object.keys(sockets).length >= maxTotalConnections
+ ) {
+ return `total connection limit (${maxTotalConnections}) reached`;
+ }
Review Comment:
Good catch. Fixed in 163acf7b81: the limit checks now count only sockets
whose underlying connection is in an active readyState (SOCKET_ACTIVE_STATES)
rather than the raw registry sizes, via small
`isSocketActive`/`activeSocketCount`/`activeChannelSocketCount` helpers.
Sockets that have closed but not yet been GC'd by `checkSockets`/`cleanChannel`
no longer count toward the total or per-channel caps. Added tests covering the
stale-closed-socket case for both limits.
##########
superset-websocket/src/index.ts:
##########
@@ -159,6 +159,37 @@ export const setLastFirehoseId = (id: string): void => {
lastFirehoseId = id;
};
+// WebSocket close code used when a connection is refused because a configured
+// connection limit has been reached (1013 = "Try Again Later").
+const CONNECTION_LIMIT_CLOSE_CODE = 1013;
+
+/**
+ * Determines whether accepting a new connection on the given channel would
+ * exceed a configured connection limit. Returns a human-readable reason when a
+ * limit is reached, or `null` when the connection is within limits.
+ *
+ * Both limits are opt-in: a value of `0` (the default) disables the check.
+ */
+export const connectionLimitReason = (channel: string): string | null => {
+ const { maxTotalConnections, maxConnectionsPerChannel } = opts;
+
+ if (
+ maxTotalConnections > 0 &&
+ Object.keys(sockets).length >= maxTotalConnections
+ ) {
+ return `total connection limit (${maxTotalConnections}) reached`;
+ }
+
+ if (maxConnectionsPerChannel > 0) {
+ const channelSize = channels[channel]?.sockets.length ?? 0;
+ if (channelSize >= maxConnectionsPerChannel) {
+ return `per-channel connection limit (${maxConnectionsPerChannel})
reached`;
+ }
Review Comment:
Good catch. Fixed in 163acf7b81: the limit checks now count only sockets
whose underlying connection is in an active readyState (SOCKET_ACTIVE_STATES)
rather than the raw registry sizes, via small
`isSocketActive`/`activeSocketCount`/`activeChannelSocketCount` helpers.
Sockets that have closed but not yet been GC'd by `checkSockets`/`cleanChannel`
no longer count toward the total or per-channel caps. Added tests covering the
stale-closed-socket case for both limits.
##########
superset-websocket/src/index.ts:
##########
@@ -159,6 +159,37 @@ export const setLastFirehoseId = (id: string): void => {
lastFirehoseId = id;
};
+// WebSocket close code used when a connection is refused because a configured
+// connection limit has been reached (1013 = "Try Again Later").
+const CONNECTION_LIMIT_CLOSE_CODE = 1013;
+
+/**
+ * Determines whether accepting a new connection on the given channel would
+ * exceed a configured connection limit. Returns a human-readable reason when a
+ * limit is reached, or `null` when the connection is within limits.
+ *
+ * Both limits are opt-in: a value of `0` (the default) disables the check.
+ */
+export const connectionLimitReason = (channel: string): string | null => {
+ const { maxTotalConnections, maxConnectionsPerChannel } = opts;
+
+ if (
+ maxTotalConnections > 0 &&
+ Object.keys(sockets).length >= maxTotalConnections
+ ) {
+ return `total connection limit (${maxTotalConnections}) reached`;
+ }
+
+ if (maxConnectionsPerChannel > 0) {
+ const channelSize = channels[channel]?.sockets.length ?? 0;
+ if (channelSize >= maxConnectionsPerChannel) {
+ return `per-channel connection limit (${maxConnectionsPerChannel})
reached`;
+ }
+ }
Review Comment:
Good catch. Fixed in 163acf7b81: the limit checks now count only sockets
whose underlying connection is in an active readyState (SOCKET_ACTIVE_STATES)
rather than the raw registry sizes, via small
`isSocketActive`/`activeSocketCount`/`activeChannelSocketCount` helpers.
Sockets that have closed but not yet been GC'd by `checkSockets`/`cleanChannel`
no longer count toward the total or per-channel caps. Added tests covering the
stale-closed-socket case for both limits.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
