From f1577c02fd3ac6e093c1ee2165dcf25cfe15f61f Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 21 Jun 2024 15:08:47 +0200 Subject: [PATCH] fix: retry jitter --- .../otlp-exporter-base/src/retryable-transport.ts | 12 +++++++++--- .../test/common/retrying-transport.test.ts | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/experimental/packages/otlp-exporter-base/src/retryable-transport.ts b/experimental/packages/otlp-exporter-base/src/retryable-transport.ts index 7c902fd8b7..e48192c1fd 100644 --- a/experimental/packages/otlp-exporter-base/src/retryable-transport.ts +++ b/experimental/packages/otlp-exporter-base/src/retryable-transport.ts @@ -21,6 +21,14 @@ const MAX_ATTEMPTS = 5; const INITIAL_BACKOFF = 1000; const MAX_BACKOFF = 5000; const BACKOFF_MULTIPLIER = 1.5; +const JITTER = 0.2; + +/** + * Get a pseudo-random jitter that falls in the range of [-JITTER, +JITTER] + */ +function getJitter() { + return Math.random() * (2 * JITTER) - JITTER; +} class RetryingTransport implements IExporterTransport { constructor(private _transport: IExporterTransport) {} @@ -38,11 +46,9 @@ class RetryingTransport implements IExporterTransport { let attempts = MAX_ATTEMPTS; let nextBackoff = INITIAL_BACKOFF; - // TODO: I'm not 100% sure this is correct, please review in-depth. while (result.status === 'retryable' && attempts > 0) { attempts--; - const upperBound = Math.min(nextBackoff, MAX_BACKOFF); - const backoff = Math.random() * upperBound; + const backoff = Math.min(nextBackoff, MAX_BACKOFF) + getJitter(); nextBackoff = nextBackoff * BACKOFF_MULTIPLIER; result = await this.retry(data, result.retryInMillis ?? backoff); } diff --git a/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts b/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts index 374d7960c5..cd7be6ebbb 100644 --- a/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts +++ b/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts @@ -152,8 +152,8 @@ describe('RetryingTransport', function () { it('does retry 5 times, then resolves as retryable', async function () { // arrange - // make random return low values so that it does not actually need to wait long for the backoff. - Math.random = sinon.stub().returns(0.001); + // make random return a negative value so that what's passed to setTimeout() is negative and therefore gets executed immediately. + Math.random = sinon.stub().returns(-Infinity); const retryResponse: ExportResponse = { status: 'retryable',