From dd94d17f4ae3de003b6d871118167138b3e4a876 Mon Sep 17 00:00:00 2001 From: eswarclynn Date: Mon, 16 Sep 2024 16:42:35 +0530 Subject: [PATCH 1/2] fix: retry timeout of 60s for reconnection --- .../src/transport/RetryScheduler.ts | 46 +++++++++++-------- .../hms-video-store/src/transport/index.ts | 4 -- .../hms-video-store/src/utils/constants.ts | 5 +- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/packages/hms-video-store/src/transport/RetryScheduler.ts b/packages/hms-video-store/src/transport/RetryScheduler.ts index 8695e5a577..ac3218b249 100644 --- a/packages/hms-video-store/src/transport/RetryScheduler.ts +++ b/packages/hms-video-store/src/transport/RetryScheduler.ts @@ -1,7 +1,7 @@ import { Dependencies as TFCDependencies, TransportFailureCategory as TFC } from './models/TransportFailureCategory'; import { TransportState } from './models/TransportState'; import { HMSException } from '../error/HMSException'; -import { MAX_TRANSPORT_RETRIES, MAX_TRANSPORT_RETRY_DELAY } from '../utils/constants'; +import { MAX_TRANSPORT_RETRY_TIME } from '../utils/constants'; import HMSLogger from '../utils/logger'; import { PromiseWithCallbacks } from '../utils/promise'; @@ -23,7 +23,7 @@ interface ScheduleTaskParams { error: HMSException; task: RetryTask; originalState: TransportState; - maxFailedRetries?: number; + maxRetryTime?: number; changeState?: boolean; } @@ -42,10 +42,10 @@ export class RetryScheduler { error, task, originalState, - maxFailedRetries = MAX_TRANSPORT_RETRIES, + maxRetryTime = MAX_TRANSPORT_RETRY_TIME, changeState = true, }: ScheduleTaskParams) { - await this.scheduleTask({ category, error, changeState, task, originalState, maxFailedRetries }); + await this.scheduleTask({ category, error, changeState, task, originalState, maxRetryTime, failedAt: Date.now() }); } reset() { @@ -65,9 +65,10 @@ export class RetryScheduler { changeState, task, originalState, - maxFailedRetries = MAX_TRANSPORT_RETRIES, + failedAt, + maxRetryTime = MAX_TRANSPORT_RETRY_TIME, failedRetryCount = 0, - }: ScheduleTaskParams & { failedRetryCount?: number }): Promise { + }: ScheduleTaskParams & { failedAt: number; failedRetryCount?: number }): Promise { HMSLogger.d(this.TAG, 'schedule: ', { category: TFC[category], error }); // First schedule call @@ -113,8 +114,9 @@ export class RetryScheduler { } } - if (failedRetryCount >= maxFailedRetries || hasFailedDependency) { - error.description += `. [${TFC[category]}] Could not recover after ${failedRetryCount} tries`; + const timeElapsedSinceError = Date.now() - failedAt; + if (timeElapsedSinceError >= maxRetryTime || hasFailedDependency) { + error.description += `. [${TFC[category]}] Could not recover after ${timeElapsedSinceError} milliseconds`; if (hasFailedDependency) { error.description += ` Could not recover all of it's required dependencies - [${(dependencies as Array) @@ -144,7 +146,7 @@ export class RetryScheduler { this.onStateChange(TransportState.Reconnecting, error); } - const delay = this.getDelayForRetryCount(category, failedRetryCount); + const delay = this.getDelayForRetryCount(category); HMSLogger.d( this.TAG, @@ -171,7 +173,10 @@ export class RetryScheduler { if (changeState && this.inProgress.size === 0) { this.onStateChange(originalState); } - HMSLogger.d(this.TAG, `schedule: [${TFC[category]}] [failedRetryCount=${failedRetryCount}] Recovered ♻️`); + HMSLogger.d( + this.TAG, + `schedule: [${TFC[category]}] [failedRetryCount=${failedRetryCount}] Recovered ♻️ after ${timeElapsedSinceError}ms`, + ); } else { await this.scheduleTask({ category, @@ -179,25 +184,26 @@ export class RetryScheduler { changeState, task, originalState, - maxFailedRetries, + maxRetryTime, + failedAt, failedRetryCount: failedRetryCount + 1, }); } } - private getBaseDelayForTask(category: TFC, n: number) { + private getDelayForRetryCount(category: TFC) { + const jitter = category === TFC.JoinWSMessageFailed ? Math.random() * 2 : Math.random(); + let delaySeconds = 0; if (category === TFC.JoinWSMessageFailed) { // linear backoff(2 + jitter for every retry) - return 2; + delaySeconds = 2 + jitter; + } else if (category === TFC.SignalDisconnect) { + delaySeconds = 1; + } else { + delaySeconds = 0; } - // exponential backoff - return Math.pow(2, n); - } - private getDelayForRetryCount(category: TFC, n: number) { - const delay = this.getBaseDelayForTask(category, n); - const jitter = category === TFC.JoinWSMessageFailed ? Math.random() * 2 : Math.random(); - return Math.round(Math.min(delay + jitter, MAX_TRANSPORT_RETRY_DELAY) * 1000); + return delaySeconds * 1000; } private async setTimeoutPromise(task: () => Promise, delay: number): Promise { diff --git a/packages/hms-video-store/src/transport/index.ts b/packages/hms-video-store/src/transport/index.ts index e244dc88b6..11b418a416 100644 --- a/packages/hms-video-store/src/transport/index.ts +++ b/packages/hms-video-store/src/transport/index.ts @@ -36,7 +36,6 @@ import { ISignalEventsObserver } from '../signal/ISignalEventsObserver'; import JsonRpcSignal from '../signal/jsonrpc'; import { ICE_DISCONNECTION_TIMEOUT, - MAX_TRANSPORT_RETRIES, PROTOCOL_SPEC, PROTOCOL_VERSION, PUBLISH_STATS_PUSH_INTERVAL, @@ -352,7 +351,6 @@ export default class HMSTransport { error, task, originalState: this.state, - maxFailedRetries: MAX_TRANSPORT_RETRIES, changeState: false, }); } else { @@ -923,7 +921,6 @@ export default class HMSTransport { error: hmsError, task, originalState: TransportState.Joined, - maxFailedRetries: 3, changeState: false, }); } else { @@ -1087,7 +1084,6 @@ export default class HMSTransport { error, task: this.retrySubscribeIceFailedTask, originalState: TransportState.Joined, - maxFailedRetries: 1, }); } } diff --git a/packages/hms-video-store/src/utils/constants.ts b/packages/hms-video-store/src/utils/constants.ts index 4e9632bc96..5f237a85d3 100644 --- a/packages/hms-video-store/src/utils/constants.ts +++ b/packages/hms-video-store/src/utils/constants.ts @@ -3,13 +3,12 @@ export const API_DATA_CHANNEL = 'ion-sfu'; export const ANALYTICS_BUFFER_SIZE = 100; /** - * Maximum number of retries that transport-layer will try + * Maximum time that transport-layer will try * before giving up on the connection and returning a failure * * Refer https://100ms.atlassian.net/browse/HMS-2369 */ -export const MAX_TRANSPORT_RETRIES = 5; -export const MAX_TRANSPORT_RETRY_DELAY = 60; +export const MAX_TRANSPORT_RETRY_TIME = 60_000; export const DEFAULT_SIGNAL_PING_TIMEOUT = 12_000; export const DEFAULT_SIGNAL_PING_INTERVAL = 3_000; From 7cc2035a6f9b6a508eeb68e23c8747f3893a6620 Mon Sep 17 00:00:00 2001 From: eswarclynn Date: Tue, 17 Sep 2024 14:50:51 +0530 Subject: [PATCH 2/2] refactor: initialise delaySeconds --- packages/hms-video-store/src/transport/RetryScheduler.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/hms-video-store/src/transport/RetryScheduler.ts b/packages/hms-video-store/src/transport/RetryScheduler.ts index ac3218b249..0feaf8c006 100644 --- a/packages/hms-video-store/src/transport/RetryScheduler.ts +++ b/packages/hms-video-store/src/transport/RetryScheduler.ts @@ -199,10 +199,7 @@ export class RetryScheduler { delaySeconds = 2 + jitter; } else if (category === TFC.SignalDisconnect) { delaySeconds = 1; - } else { - delaySeconds = 0; } - return delaySeconds * 1000; }