From a9490c30898adc37faa763b2ee2e9a010969e5c2 Mon Sep 17 00:00:00 2001 From: Doug Reeder Date: Wed, 21 Feb 2024 16:21:13 -0500 Subject: [PATCH] Increases anti-lock timeout to avoid 'Locked !?!?' file-lock errors (see #90) (#112) * Armadietto is prone to 'Locked !?!?' file-lock errors in real world situations: the timeout of 200ms is insufficient when batch putting or getting. Provide file lock timeout flexibility and make gets non-locking: just cross-checking etag before and after. * Skip node 12. * Add hosted load test link. * Sets default lock timeout to 30 seconds --------- Co-authored-by: Jakub Ner --- .github/workflows/publish.yml | 2 +- .github/workflows/test-and-lint.yml | 1 - README.md | 11 + bin/armadietto.js | 2 +- bin/dev-conf.json | 2 + contrib/openwrt/README.md | 2 +- example/index.html | 4 +- example/load.html | 345 +++++++++++++++++++++++++ lib/stores/file_tree.js | 43 ++- package.json | 2 +- spec/stores/file_tree_lockfree_spec.js | 76 ++++++ 11 files changed, 477 insertions(+), 13 deletions(-) create mode 100644 example/load.html create mode 100644 spec/stores/file_tree_lockfree_spec.js diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index be504871..f3144253 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -21,7 +21,7 @@ jobs: strategy: matrix: # Support LTS versions based on https://nodejs.org/en/about/releases/ - node-version: ['12', '14', '16'] + node-version: ['18', '20', '21'] steps: - uses: actions/checkout@v2 - uses: actions/setup-node@v2 diff --git a/.github/workflows/test-and-lint.yml b/.github/workflows/test-and-lint.yml index 0b8cf179..5af17ca9 100644 --- a/.github/workflows/test-and-lint.yml +++ b/.github/workflows/test-and-lint.yml @@ -24,4 +24,3 @@ jobs: run: npm run lint - name: Run tests run: npm test - diff --git a/README.md b/README.md index 36064ac1..dbdcf6c3 100644 --- a/README.md +++ b/README.md @@ -188,6 +188,17 @@ const server = new Armadietto({ server.boot(); ``` +## Lock file contention + +The data-access locking mechanism is lock-file based. + +You may need to tune the lock-file timeouts in your configuration: + +- *lock_timeout_ms* - millis to wait for lock file to be available +- *lock_stale_after_ms* - millis to wait to deem lockfile stale + +To tune run the [hosted RS load test](https://overhide.github.io/armadietto/example/load.html) or follow instructions in [example/README.md](example/README.md) for local setup and subsequently run [example/load.html](example/load.html) off of `npm run serve` therein. + ## Debugging an installation Set the environment `DEBUG` to enable logging. For example `DEBUG=true armadietto -c /etc/armadietto/conf.json` diff --git a/bin/armadietto.js b/bin/armadietto.js index 5a51548e..f34e2549 100755 --- a/bin/armadietto.js +++ b/bin/armadietto.js @@ -53,7 +53,7 @@ const remoteStorageServer = { } process.umask(0o077); - const store = new Armadietto.FileTree({ path: conf.storage_path }); + const store = new Armadietto.FileTree({ path: conf.storage_path, lock_timeout_ms: conf.lock_timeout_ms, lock_stale_after_ms: conf.lock_stale_after_ms }); const server = new Armadietto({ basePath: conf.basePath, store, diff --git a/bin/dev-conf.json b/bin/dev-conf.json index dc65016b..cb321c3f 100644 --- a/bin/dev-conf.json +++ b/bin/dev-conf.json @@ -2,6 +2,8 @@ "basePath": "", "allow_signup": true, "storage_path": "./dev-storage", + "lock_timeout_ms": 30000, + "lock_stale_after_ms": 60000, "cache_views": true, "http": { "host": "127.0.0.1", diff --git a/contrib/openwrt/README.md b/contrib/openwrt/README.md index ac918498..ad99572b 100644 --- a/contrib/openwrt/README.md +++ b/contrib/openwrt/README.md @@ -14,7 +14,7 @@ See [TLS certificates for a server](https://openwrt.org/docs/guide-user/services We assume that you already issued a cert To store data you need to mount a disk. -See [Quick Start for Adding a USB drive](https://openwrt.org/docs/guide-user/storage/usb-drives-quickstart). +See [Quick Start for Adding a USB drive](https://openwrt.org/docs/guide-user/storage/usb-drives-quickstart). In the example it's mounted to /mnt/disk/ Next we need to create a folder to store the user data. Login to OpenWrt with `ssh root@192.168.1.1` and execute: diff --git a/example/index.html b/example/index.html index 567bca91..b08c7b59 100644 --- a/example/index.html +++ b/example/index.html @@ -4,8 +4,8 @@ RemoteStorage Demo - - + + diff --git a/example/load.html b/example/load.html new file mode 100644 index 00000000..0f4bde58 --- /dev/null +++ b/example/load.html @@ -0,0 +1,345 @@ + + + + + + RemoteStorage Load Test + + + + + + + + + + + + +

|<---->| Cannot view, screen to narrow, please revisit on a wider device.

+
+
+ +
+ + + + +
+

+ + + + +

+
+ + +
+
LOAD TEST over remoteStorage.js

+ remoteStorage.js abstracted API calls to the server. +

+ Use widget (above) to retrieve the token. +

+ Check log-pane for results. +

+

+ + +

+

+

+

+

+
+ +
+
+
+
+
+
+ +
+
+
+ + + + diff --git a/lib/stores/file_tree.js b/lib/stores/file_tree.js index 20be3080..08aa0f97 100644 --- a/lib/stores/file_tree.js +++ b/lib/stores/file_tree.js @@ -16,12 +16,14 @@ const writeFile = promisify(fs.writeFile); class FileTree { constructor (options) { this._dir = path.resolve(options.path); + this._timeout = options.lock_timeout_ms ?? 30000; + this._stale = options.lock_stale_after_ms ?? 60000; } async _lock (username) { const lockPath = path.join(this._dir, username.substr(0, 2), username, '.lock'); try { - await lock(lockPath, { wait: 200 }); + await lock(lockPath, { wait: this._timeout, stale: this._stale }); } catch (e) { const err = new Error('Locked !?!?' + e.toString()); getLogger().error('lock failed:', err); @@ -157,13 +159,43 @@ class FileTree { return versions.find(version => modified === version.trim().replace(/"/g, '')); } + async _etagMatchOrThrow (originalEtag, username, pathname, isdir) { + const metadata = await this.readMeta(username, pathname, isdir); + if (metadata.ETag !== originalEtag) { + throw new Error('ETag mismatch'); + } + } + + async _delay (t) { + return new Promise(resolve => setTimeout(resolve.bind(null), t)); + } + async get (username, pathname, versions, head = false) { + const startTime = Date.now(); + + for (;;) { + try { + return await this._lockfree_get(username, pathname, versions, head); + } catch (e) { + if (e.message === 'ETag mismatch') { + const nowTime = Date.now(); + if (nowTime - startTime > this._timeout) { + throw e; + } + await this._delay(100); + } else { + throw e; + } + } + } + } + + async _lockfree_get (username, pathname, versions, head) { versions = versions && versions.split(','); const isdir = /\/$/.test(pathname); const basename = decodeURI(path.basename(pathname)) + (isdir ? '/' : ''); const datapath = this.dataPath(username, pathname); - await this._lock(username); const metadata = await this.readMeta(username, pathname, isdir); // resource exists? @@ -171,25 +203,24 @@ class FileTree { if (!isdir && !metadata.ETag) ret = { item: null }; if (!isdir && !metadata.items[basename]) ret = { item: null }; if (ret) { - await this._unlock(username); return ret; } // has client the same version of this resource? const currentETag = isdir ? metadata.ETag : metadata.items[basename].ETag; if (this._versionMatch(versions, currentETag)) { - await this._unlock(username); + await this._etagMatchOrThrow(metadata.ETag, username, pathname, isdir); return { item: metadata, versionMatch: true }; } // dir listing if (isdir) { - await this._unlock(username); + await this._etagMatchOrThrow(metadata.ETag, username, pathname, isdir); return { item: metadata }; } else { // do not include content on head request const blob = head ? '' : await readFile(datapath); - await this._unlock(username); + await this._etagMatchOrThrow(metadata.ETag, username, pathname, isdir); if (blob === null) return { item: null }; const item = metadata.items[basename]; diff --git a/package.json b/package.json index a4497b91..5f12f134 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "license": "MIT", "version": "0.2.0", "engines": { - "node": ">=14.0" + "node": ">=18.0" }, "bin": { "armadietto": "./bin/armadietto.js" diff --git a/spec/stores/file_tree_lockfree_spec.js b/spec/stores/file_tree_lockfree_spec.js new file mode 100644 index 00000000..6b92c887 --- /dev/null +++ b/spec/stores/file_tree_lockfree_spec.js @@ -0,0 +1,76 @@ +/* eslint-env mocha, chai, node */ +const path = require('path'); +const rmrf = require('rimraf'); +const chai = require('chai'); +const expect = chai.expect; +const FileTree = require('../../lib/stores/file_tree'); + +describe('FileTree store lockfree get', async () => { + const store = new FileTree({ path: path.join(__dirname, '/../../tmp/store'), lock_timeout_ms: 1000 }); + + before((done) => { + (async () => { + rmrf(path.join(__dirname, '/../../tmp/store'), () => {}); + await store.createUser({ username: 'boris', email: 'boris@example.com', password: 'zipwire' }); + done(); + })(); + }); + + store.__readMeta = store.readMeta; + + after((done) => { + (async () => { + rmrf(path.join(__dirname, '/../../tmp/store'), () => {}); + done(); + })(); + }); + + const getReadMetaInterrupted = (numberInterruptions) => { + let callNum = 0; + store.readMeta = async (username, pathname, isdir) => { + const metadata = await store.__readMeta(username, pathname, isdir); + if (callNum < numberInterruptions) { + metadata.ETag = `${callNum}`; + metadata.items.zipwire.ETag = `${callNum}`; + } + callNum++; + return metadata; + }; + }; + + it('returns the value in the response', async () => { + await store.put('boris', '/photos/zipwire', 'image/poster', Buffer.from('vertibo'), null); + const { item } = await store.get('boris', '/photos/zipwire', null); + expect(item.value).to.be.deep.equal(Buffer.from('vertibo')); + }); + + it('returns the value in the response after one interruption', async () => { + await store.put('boris', '/photos/zipwire', 'image/poster', Buffer.from('vertibo'), null); + + getReadMetaInterrupted(1); + + const { item } = await store.get('boris', '/photos/zipwire', null); + expect(item.value).to.be.deep.equal(Buffer.from('vertibo')); + }); + + it('returns the value in the response after two interruption', async () => { + await store.put('boris', '/photos/zipwire', 'image/poster', Buffer.from('vertibo'), null); + + getReadMetaInterrupted(3); + + const { item } = await store.get('boris', '/photos/zipwire', null); + expect(item.value).to.be.deep.equal(Buffer.from('vertibo')); + }); + + it('gets exception after three interruption', async () => { + await store.put('boris', '/photos/zipwire', 'image/poster', Buffer.from('vertibo'), null); + + getReadMetaInterrupted(50); + + try { + await store.get('boris', '/photos/zipwire', null); + } catch (e) { + expect(e.message).to.be.equal('ETag mismatch'); + } + }); +});