This is an automated email from the ASF dual-hosted git repository.
xuanwo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git
The following commit(s) were added to refs/heads/main by this push:
new 3cd08b666 feat(bindings/nodejs): add retry layer (#3484)
3cd08b666 is described below
commit 3cd08b666c67e61da1272ee90a119b4b012c7f29
Author: Suyan <[email protected]>
AuthorDate: Tue Nov 14 20:54:14 2023 +0800
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]>
---
.github/workflows/bindings_nodejs.yml | 9 +-
bindings/nodejs/generated.js | 4 +-
bindings/nodejs/index.d.ts | 78 +++++++++++++++
bindings/nodejs/index.js | 5 +-
bindings/nodejs/src/lib.rs | 138 ++++++++++++++++++++++++++-
bindings/nodejs/tests/suites/async.suite.mjs | 2 +-
bindings/nodejs/tests/suites/index.mjs | 10 +-
bindings/nodejs/tests/suites/sync.suite.mjs | 2 +-
fixtures/alluxio/docker-compose-alluxio.yml | 6 +-
9 files changed, 240 insertions(+), 14 deletions(-)
diff --git a/.github/workflows/bindings_nodejs.yml
b/.github/workflows/bindings_nodejs.yml
index c9f176800..6891109f8 100644
--- 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
index 915128b86..2b40ec25e 100644
--- 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
index 112ece450..0d0455b0b 100644
--- 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
index 1200443d5..c1af66747 100644
--- 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
index 7882be4cb..e994e1fac 100644
--- 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
index 8214c1b17..9a68f804e 100644
--- 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
index cd8998009..a67467f28 100644
--- 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
index 473d121c6..5e73444d3 100644
--- 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
index d725fe8c3..6fe399dcb 100644
--- 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
-
\ No newline at end of file
+
