Merge pull request #226 from influxdata/feat/retry_delay_strategy

chore(core): improve retry strategy and tests
influxdata · Jul 31, 2020 · b36d683 · b36d683
2 parents 3febb4c + 1e26109
commit b36d683
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ### Features
 
 1. [#219](https://github.com/influxdata/influxdb-client-js/pull/219): Sanitize arrays in parameterized flux queries.
+1. [#226](https://github.com/influxdata/influxdb-client-js/pull/226): Improve retry strategy.
 
 ### Documentation
 

diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts
@@ -5,9 +5,10 @@ export interface RetryDelayStrategy {
   /**
    * Returns delay for a next retry
    * @param error - reason for retrying
+   * @param failedAttempts - a count of already failed attempts, 1 being the first
    * @returns milliseconds to wait before retrying
    */
-  nextDelay(error?: Error): number
+  nextDelay(error?: Error, failedAttempts?: number): number
   /** Implementation should reset its state, this is mandatory to call upon success.  */
   success(): void
 }

diff --git a/packages/core/src/impl/WriteApiImpl.ts b/packages/core/src/impl/WriteApiImpl.ts
@@ -134,12 +134,13 @@ export default class WriteApiImpl implements WriteApi, PointSettings {
       return new Promise<void>((resolve, reject) => {
         this.transport.send(this.httpPath, lines.join('\n'), this.sendOptions, {
           error(error: Error): void {
+            const failedAttempts = self.writeOptions.maxRetries + 2 - attempts
             // call the writeFailed listener and check if we can retry
             const onRetry = self.writeOptions.writeFailed.call(
               self,
               error,
               lines,
-              self.writeOptions.maxRetries + 2 - attempts
+              failedAttempts
             )
             if (onRetry) {
               onRetry.then(resolve, reject)
@@ -159,7 +160,7 @@ export default class WriteApiImpl implements WriteApi, PointSettings {
               self.retryBuffer.addLines(
                 lines,
                 attempts - 1,
-                self.retryStrategy.nextDelay(error)
+                self.retryStrategy.nextDelay(error, failedAttempts)
               )
               reject(error)
               return

diff --git a/packages/core/src/impl/retryStrategy.ts b/packages/core/src/impl/retryStrategy.ts
@@ -17,15 +17,29 @@ export class RetryStrategyImpl implements RetryDelayStrategy {
     this.success()
   }
 
-  nextDelay(error?: Error): number {
+  nextDelay(error?: Error, failedAttempts?: number): number {
     const delay = getRetryDelay(error)
     if (delay && delay > 0) {
       return Math.min(
         delay + Math.round(Math.random() * this.options.retryJitter),
         this.options.maxRetryDelay
       )
     } else {
-      if (this.currentDelay) {
+      let delay = this.currentDelay
+      if (failedAttempts && failedAttempts > 0) {
+        // compute delay
+        delay = this.options.minRetryDelay
+        for (let i = 1; i < failedAttempts; i++) {
+          delay = delay * 2
+          if (delay >= this.options.maxRetryDelay) {
+            break
+          }
+        }
+        return (
+          Math.min(Math.max(delay, 1), this.options.maxRetryDelay) +
+          Math.round(Math.random() * this.options.retryJitter)
+        )
+      } else if (this.currentDelay) {
         this.currentDelay = Math.min(
           Math.max(this.currentDelay * 2, 1) +
             Math.round(Math.random() * this.options.retryJitter),

diff --git a/packages/core/test/unit/impl/retryStrategy.test.ts b/packages/core/test/unit/impl/retryStrategy.test.ts
@@ -59,6 +59,26 @@ describe('RetryStrategyImpl', () => {
       expect(x).to.not.be.greaterThan(1000)
     })
   })
+  it('generates exponential delays with failedAttempts', () => {
+    const subject = new RetryStrategyImpl({
+      minRetryDelay: 100,
+      maxRetryDelay: 1000,
+      retryJitter: 10,
+    })
+    const values = [1, 2, 3, 4, 5, 6].reduce((acc, val) => {
+      acc.push(subject.nextDelay(new Error(), val))
+      return acc
+    }, [] as number[])
+    expect(values).to.have.length(6)
+    values.forEach((x, i) => {
+      if (i > 0) {
+        expect(Math.max(Math.trunc(x / 100), 10)).to.not.be.lessThan(
+          Math.max(Math.trunc(values[i - 1] / 100), 10)
+        )
+      }
+      expect(x).to.not.be.greaterThan(1000 + 10)
+    })
+  })
   it('generates default jittered delays', () => {
     const subject = new RetryStrategyImpl({
       minRetryDelay: 100,

diff --git a/packages/core/test/unit/query/flux.test.ts b/packages/core/test/unit/query/flux.test.ts
@@ -112,6 +112,9 @@ describe('Flux Values', () => {
       {value: 'abc${val}def', flux: '"abc\\${val}def"'},
       {value: 'abc$', flux: '"abc$"'},
       {value: 'a"$d', flux: '"a\\"$d"'},
+      {value: [], flux: '[]'},
+      {value: ['a"$d'], flux: '["a\\"$d"]'},
+      {value: Symbol('thisSym'), flux: `"${Symbol('thisSym').toString()}"`},
     ]
     pairs.forEach(pair => {
       it(`converts ${JSON.stringify(String(pair.value))} to '${