feat(bindings/nodejs): add retry layer (#3484)

* feat(bindings/nodejs): add retry layer Signed-off-by: suyanhanx <[email protected]> * Try Signed-off-by: Xuanwo <[email protected]> * Remove compat-mode Signed-off-by: Xuanwo <[email protected]> * polish API Signed-off-by: Xuanwo <[email protected]> * fill Signed-off-by: suyanhanx <[email protected]> * polish tests Signed-off-by: suyanhanx <[email protected]> * fmt Signed-off-by: suyanhanx <[email protected]> * revert test job & remove useless clippy allow Signed-off-by: suyanhanx <[email protected]> * add some doc Signed-off-by: suyanhanx <[email protected]> * commit type file Signed-off-by: suyanhanx <[email protected]> --------- Signed-off-by: suyanhanx <[email protected]> Signed-off-by: Xuanwo <[email protected]> Co-authored-by: Xuanwo <[email protected]>
apache · Nov 14, 2023 · 3cd08b6 · 3cd08b6
1 parent 6d46e38
commit 3cd08b6
Show file tree

Hide file tree

Showing 9 changed files with 240 additions and 14 deletions.
diff --git a/.github/workflows/bindings_nodejs.yml b/.github/workflows/bindings_nodejs.yml
@@ -48,8 +48,6 @@ jobs:
       - uses: actions/checkout@v4
       - name: Setup Rust toolchain
         uses: ./.github/actions/setup
-        with:
-          need-nextest: true
       - name: Setup node
         uses: actions/setup-node@v4
         with:
@@ -65,11 +63,14 @@ jobs:
       - name: Check format
         run: yarn run prettier --check .
 
+      - name: Build
+        run: yarn build
+
       - name: Check diff
         run: git diff --exit-code
 
-      - name: Unit test
-        run: cargo nextest run --no-fail-fast
+      - name: Test
+        run: cargo test --no-fail-fast
 
   linux:
     runs-on: ubuntu-latest

diff --git a/bindings/nodejs/generated.js b/bindings/nodejs/generated.js
@@ -271,10 +271,12 @@ if (!nativeBinding) {
   throw new Error(`Failed to load native binding`)
 }
 
-const { Operator, Entry, Metadata, Lister, BlockingLister } = nativeBinding
+const { Operator, Entry, Metadata, Lister, BlockingLister, Layer, RetryLayer } = nativeBinding
 
 module.exports.Operator = Operator
 module.exports.Entry = Entry
 module.exports.Metadata = Metadata
 module.exports.Lister = Lister
 module.exports.BlockingLister = BlockingLister
+module.exports.Layer = Layer
+module.exports.RetryLayer = RetryLayer
diff --git a/bindings/nodejs/index.d.ts b/bindings/nodejs/index.d.ts
@@ -22,6 +22,12 @@
 
 /* auto-generated by NAPI-RS */
 
+export class ExternalObject<T> {
+  readonly '': {
+    readonly '': unique symbol
+    [K: symbol]: T
+  }
+}
 export interface PresignedRequest {
   /** HTTP method of this request. */
   method: string
@@ -394,6 +400,8 @@ export class Operator {
    * ```
    */
   presignStat(path: string, expires: number): Promise<PresignedRequest>
+  /** Add a layer to this operator. */
+  layer(layer: ExternalObject<Layer>): this
 }
 export class Entry {
   /** Return the path of this entry. */
@@ -435,3 +443,73 @@ export class Lister {
 export class BlockingLister {
   next(): Entry | null
 }
+/** A public layer wrapper */
+export class Layer { }
+/**
+ * Retry layer
+ *
+ * Add retry for temporary failed operations.
+ *
+ * # Notes
+ *
+ * This layer will retry failed operations when [`Error::is_temporary`]
+ * returns true. If operation still failed, this layer will set error to
+ * `Persistent` which means error has been retried.
+ *
+ * `write` and `blocking_write` don't support retry so far, visit [this issue](https://github.com/apache/incubator-opendal/issues/1223) for more details.
+ *
+ * # Examples
+ *
+ * ```javascript
+ * const op = new Operator("file", { root: "/tmp" })
+ *
+ * const retry = new RetryLayer();
+ * retry.max_times = 3;
+ * retry.jitter = true;
+ *
+ * op.layer(retry.build());
+ *```
+ */
+export class RetryLayer {
+  constructor()
+  /**
+   * Set jitter of current backoff.
+   *
+   * If jitter is enabled, ExponentialBackoff will add a random jitter in `[0, min_delay)
+   * to current delay.
+   */
+  set jitter(v: boolean)
+  /**
+   * Set max_times of current backoff.
+   *
+   * Backoff will return `None` if max times is reaching.
+   */
+  set maxTimes(v: number)
+  /**
+   * Set factor of current backoff.
+   *
+   * # Panics
+   *
+   * This function will panic if input factor smaller than `1.0`.
+   */
+  set factor(v: number)
+  /**
+   * Set max_delay of current backoff.
+   *
+   * Delay will not increasing if current delay is larger than max_delay.
+   *
+   * # Notes
+   *
+   * - The unit of max_delay is millisecond.
+   */
+  set maxDelay(v: number)
+  /**
+   * Set min_delay of current backoff.
+   *
+   * # Notes
+   *
+   * - The unit of min_delay is millisecond.
+   */
+  set minDelay(v: number)
+  build(): ExternalObject<Layer>
+}
diff --git a/bindings/nodejs/index.js b/bindings/nodejs/index.js
@@ -19,6 +19,9 @@
 
 /// <reference types="node" />
 
-const { Operator } = require('./generated.js')
+const { Operator, RetryLayer } = require('./generated.js')
 
 module.exports.Operator = Operator
+module.exports.layers = {
+  RetryLayer,
+}
diff --git a/bindings/nodejs/src/lib.rs b/bindings/nodejs/src/lib.rs
@@ -24,7 +24,6 @@ use std::time::Duration;
 
 use futures::TryStreamExt;
 use napi::bindgen_prelude::*;
-use napi::tokio;
 
 #[napi]
 pub struct Operator(opendal::Operator);
@@ -685,6 +684,143 @@ impl PresignedRequest {
     }
 }
 
+pub trait NodeLayer: Send + Sync {
+    fn layer(&self, op: opendal::Operator) -> opendal::Operator;
+}
+
+/// A public layer wrapper
+#[napi]
+pub struct Layer {
+    inner: Box<dyn NodeLayer>,
+}
+
+#[napi]
+impl Operator {
+    /// Add a layer to this operator.
+    #[napi]
+    pub fn layer(&self, layer: External<Layer>) -> Result<Self> {
+        Ok(Self(layer.inner.layer(self.0.clone())))
+    }
+}
+
+impl NodeLayer for opendal::layers::RetryLayer {
+    fn layer(&self, op: opendal::Operator) -> opendal::Operator {
+        op.layer(self.clone())
+    }
+}
+
+/// Retry layer
+///
+/// Add retry for temporary failed operations.
+///
+/// # Notes
+///
+/// This layer will retry failed operations when [`Error::is_temporary`]
+/// returns true. If operation still failed, this layer will set error to
+/// `Persistent` which means error has been retried.
+///
+/// `write` and `blocking_write` don't support retry so far, visit [this issue](https://github.com/apache/incubator-opendal/issues/1223) for more details.
+///
+/// # Examples
+///
+/// ```javascript
+/// const op = new Operator("file", { root: "/tmp" })
+///
+/// const retry = new RetryLayer();
+/// retry.max_times = 3;
+/// retry.jitter = true;
+///
+/// op.layer(retry.build());
+///```
+#[derive(Default)]
+#[napi]
+pub struct RetryLayer {
+    jitter: bool,
+    max_times: Option<u32>,
+    factor: Option<f64>,
+    max_delay: Option<f64>,
+    min_delay: Option<f64>,
+}
+
+#[napi]
+impl RetryLayer {
+    #[napi(constructor)]
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    /// Set jitter of current backoff.
+    ///
+    /// If jitter is enabled, ExponentialBackoff will add a random jitter in `[0, min_delay)
+    /// to current delay.
+    #[napi(setter)]
+    pub fn jitter(&mut self, v: bool) {
+        self.jitter = v;
+    }
+
+    /// Set max_times of current backoff.
+    ///
+    /// Backoff will return `None` if max times is reaching.
+    #[napi(setter)]
+    pub fn max_times(&mut self, v: u32) {
+        self.max_times = Some(v);
+    }
+
+    /// Set factor of current backoff.
+    ///
+    /// # Panics
+    ///
+    /// This function will panic if input factor smaller than `1.0`.
+    #[napi(setter)]
+    pub fn factor(&mut self, v: f64) {
+        self.factor = Some(v);
+    }
+
+    /// Set max_delay of current backoff.
+    ///
+    /// Delay will not increasing if current delay is larger than max_delay.
+    ///
+    /// # Notes
+    ///
+    /// - The unit of max_delay is millisecond.
+    #[napi(setter)]
+    pub fn max_delay(&mut self, v: f64) {
+        self.max_delay = Some(v);
+    }
+
+    /// Set min_delay of current backoff.
+    ///
+    /// # Notes
+    ///
+    /// - The unit of min_delay is millisecond.
+    #[napi(setter)]
+    pub fn min_delay(&mut self, v: f64) {
+        self.min_delay = Some(v);
+    }
+
+    #[napi]
+    pub fn build(&self) -> External<Layer> {
+        let mut l = opendal::layers::RetryLayer::default();
+        if self.jitter {
+            l = l.with_jitter();
+        }
+        if let Some(max_times) = self.max_times {
+            l = l.with_max_times(max_times as usize);
+        }
+        if let Some(factor) = self.factor {
+            l = l.with_factor(factor as f32);
+        }
+        if let Some(max_delay) = self.max_delay {
+            l = l.with_max_delay(Duration::from_millis(max_delay as u64));
+        }
+        if let Some(min_delay) = self.min_delay {
+            l = l.with_min_delay(Duration::from_millis(min_delay as u64));
+        }
+
+        External::new(Layer { inner: Box::new(l) })
+    }
+}
+
 fn format_napi_error(err: opendal::Error) -> Error {
     Error::from_reason(format!("{}", err))
 }
diff --git a/bindings/nodejs/tests/suites/async.suite.mjs b/bindings/nodejs/tests/suites/async.suite.mjs
@@ -28,7 +28,7 @@ export function run(operator) {
     try {
       await operator.stat(filename)
     } catch (error) {
-      assert.ok(error.message.includes('NotFound'))
+      assert.include(error.message, 'NotFound')
     }
   })
 }
diff --git a/bindings/nodejs/tests/suites/index.mjs b/bindings/nodejs/tests/suites/index.mjs
@@ -18,7 +18,7 @@
  */
 
 import { describe } from 'vitest'
-import { Operator } from '../../index.js'
+import { Operator, layers } from '../../index.js'
 import { checkRandomRootEnabled, generateRandomRoot, loadConfigFromEnv } from '../utils.mjs'
 
 import { run as AsyncIOTestRun } from './async.suite.mjs'
@@ -36,7 +36,13 @@ export function runner(testName, scheme) {
     config.root = generateRandomRoot(config.root)
   }
 
-  const operator = scheme ? new Operator(scheme, config) : null
+  let operator = scheme ? new Operator(scheme, config) : null
+
+  let retryLayer = new layers.RetryLayer()
+  retryLayer.jitter = true
+  retryLayer.maxTimes = 4
+
+  operator = operator.layer(retryLayer.build())
 
   describe.skipIf(!operator)(testName, () => {
     AsyncIOTestRun(operator)

diff --git a/bindings/nodejs/tests/suites/sync.suite.mjs b/bindings/nodejs/tests/suites/sync.suite.mjs
@@ -28,7 +28,7 @@ export function run(operator) {
     try {
       operator.statSync(filename)
     } catch (error) {
-      assert.ok(error.message.includes('NotFound'))
+      assert.include(error.message, 'NotFound')
     }
   })
 }
diff --git a/fixtures/alluxio/docker-compose-alluxio.yml b/fixtures/alluxio/docker-compose-alluxio.yml
@@ -23,7 +23,7 @@ services:
       - 19999:19999
       - 19998:19998
     environment:
-      ALLUXIO_JAVA_OPTS:  -Dalluxio.master.hostname=alluxio-master -Dalluxio.master.mount.table.root.ufs=/opt/alluxio/underFSStorage
+      ALLUXIO_JAVA_OPTS: -Dalluxio.master.hostname=alluxio-master -Dalluxio.master.mount.table.root.ufs=/opt/alluxio/underFSStorage
     command: master
     networks:
       - alluxio_network
@@ -55,7 +55,7 @@ services:
       - 30000:30000
     shm_size: 1gb
     environment:
-      ALLUXIO_JAVA_OPTS:  -Dalluxio.worker.ramdisk.size=1G     -Dalluxio.master.hostname=alluxio-master  -Dalluxio.worker.hostname=alluxio-worker
+      ALLUXIO_JAVA_OPTS: -Dalluxio.worker.ramdisk.size=1G     -Dalluxio.master.hostname=alluxio-master  -Dalluxio.worker.hostname=alluxio-worker
     command: worker
     networks:
       - alluxio_network
@@ -69,4 +69,4 @@ services:
 networks:
   alluxio_network:
     driver: bridge
-    
+