test: add separate-thread concurrency test #305

SgtPooki · 2024-04-19T18:07:45Z

Basically spawned from #285 (comment)

This PR adds a test to both datastore-fs and blockstore-fs that validates puts are somewhat concurrent safe across multiple threads.

SgtPooki

self review

SgtPooki · 2024-04-19T18:10:38Z

packages/blockstore-fs/src/index.ts

@@ -48,7 +48,6 @@ async function writeFile (file: string, contents: Uint8Array): Promise<void> {
      // attempts to write the same block by two different function calls
      return
    }


should add the newline back here.

SgtPooki · 2024-04-19T18:11:29Z

packages/blockstore-fs/src/index.ts

@@ -37,7 +37,7 @@ async function writeFile (file: string, contents: Uint8Array): Promise<void> {
  try {
    await writeAtomic(file, contents)
  } catch (err: any) {
-    if (err.code === 'EPERM' && err.syscall === 'rename') {
+    if (err.syscall === 'rename' && ['ENOENT', 'EPERM'].includes(err.code)) {


FYI that on my mac, an ENOENT Error was firing on the rename call.

example:

blockstore-fs: Error putting block [Error: ENOENT: no such file or directory, rename '/var/folders/bl/_gl5_59s11v7qz5ysd6bfgb00000gn/T/test-0.8216405349064326/LY/.63718.93' -> '/var/folders/bl/_gl5_59s11v7qz5ysd6bfgb00000gn/T/test-0.8216405349064326/LY/BCIQPGZJ6QLZOFG3OP45NLMSJUWGJCO72QQKHLDTB6FXIB6BDSLRQYLY.data'] { blockstore-fs: errno: -2, blockstore-fs: code: 'ERR_PUT_FAILED', blockstore-fs: syscall: 'rename', blockstore-fs: path: '/var/folders/bl/_gl5_59s11v7qz5ysd6bfgb00000gn/T/test-0.8216405349064326/LY/.63718.93', blockstore-fs: dest: '/var/folders/bl/_gl5_59s11v7qz5ysd6bfgb00000gn/T/test-0.8216405349064326/LY/BCIQPGZJ6QLZOFG3OP45NLMSJUWGJCO72QQKHLDTB6FXIB6BDSLRQYLY.data' blockstore-fs: }

I think if we validate that the final file exists and is F_OK, W_OK as we were doing for EPERM, this is safe.

I also thought about checking that error.dest matches, but it seems like that is effectively what we're doing with fs.access, so this should work

SgtPooki · 2024-04-19T18:11:59Z

packages/blockstore-fs/test/fixtures/writer-worker.ts

+      return true
+    } catch (err) {
+      // eslint-disable-next-line no-console
+      console.error('Error opening blockstore', err)


so we can get some error information in testing environment without dealing with logger? i can add in a logger if desired.

~~rethrow the error here.~~ done.

packages/blockstore-fs/test/fixtures/writer-worker.ts

SgtPooki · 2024-04-19T18:13:53Z

packages/datastore-fs/src/index.ts

@@ -37,7 +37,7 @@ async function writeFile (path: string, contents: Uint8Array): Promise<void> {
  try {
    await writeAtomic(path, contents)
  } catch (err: any) {
-    if (err.code === 'EPERM' && err.syscall === 'rename') {
+    if (err.syscall === 'rename' && ['ENOENT', 'EPERM'].includes(err.code)) {


see comment from same changes in blockstore-fs: #305 (comment)

looks like we're also getting an EPERM on 'open' with fast-atomic-write on windows:

Error: EPERM: operation not permitted, open 'C:\Users\RUNNER~1\AppData\Local\Temp\test-0.9268640174546598\LY.6280.3'

https://github.com/ipfs/js-stores/actions/runs/8757918340/job/24047367236?pr=305#step:5:687

This is not failing on windows with steno: #285

packages/datastore-fs/test/fixtures/writer-worker.ts

SgtPooki · 2024-04-20T00:33:05Z

Note that https://github.com/ipfs/js-stores/actions/runs/8757918340/job/24037687392?pr=305#step:5:645 is failing here for windows, but passing in #285.

going to retry the job. new job at https://github.com/ipfs/js-stores/actions/runs/8757918340?pr=305

SgtPooki · 2024-04-22T18:14:28Z

Note that ipfs/js-stores/actions/runs/8757918340/job/24037687392?pr=305#step:5:645 is failing here for windows, but passing in #285.

going to retry the job. new job at ipfs/js-stores/actions/runs/8757918340?pr=305

findings in #305 (comment).. seem's like fast-write-atomic steps on itself with temp files when ran in a separate concurrent thread.

fixes #284 fast-write-atomic hasn't been updated in 5 years, is CJS, and is slower than steno (updated 2 months ago). Benchmarks for various content-types & libraries (though we only use Uint8Arrays) can be found at https://github.com/SgtPooki/fast-write-atomic#benchmarks However, there may be further room for improvement by moving to [fs.createWriteStream](https://nodejs.org/api/fs.html#fscreatewritestreampath-options) ``` ╰─ ✔ ❯ hyperfine --parameter-list branch 284-chore-replace-fast-write-atomic-with-steno,main --setup "git switch {branch} && npm run reset && npm i && npm run build" --runs 20 -w 1 "npm run test:node" Benchmark 1: npm run test:node (branch = 284-chore-replace-fast-write-atomic-with-steno) Time (mean ± σ): 27.212 s ± 0.832 s [User: 34.810 s, System: 6.051 s] Range (min … max): 25.927 s … 29.324 s 20 runs Benchmark 2: npm run test:node (branch = main) Time (mean ± σ): 42.971 s ± 0.637 s [User: 35.297 s, System: 7.534 s] Range (min … max): 42.178 s … 44.796 s 20 runs Summary npm run test:node (branch = 284-chore-replace-fast-write-atomic-with-steno) ran 1.58 ± 0.05 times faster than npm run test:node (branch = main) ``` --- ### Updated benchmarks of `npm run test` as of 2024-04-19 ``` ╭─    ~/code/work/protocol.ai/ipfs/js-stores    main ?1 ╰─ ✔ ❯ hyperfine --parameter-list branch main,test/not-same-event-loop-concurrency,284-chore-replace-fast-write-atomic-with-steno --setup "git switch {branch} && npm run reset && npm i && npm run build && cd packages/datastore-fs" "npm run test" Benchmark 1: npm run test (branch = main) Time (mean ± σ): 99.415 s ± 2.918 s [User: 69.659 s, System: 23.361 s] Range (min … max): 96.134 s … 105.200 s 10 runs Benchmark 2: npm run test (branch = test/not-same-event-loop-concurrency) Time (mean ± σ): 103.456 s ± 3.186 s [User: 74.442 s, System: 25.261 s] Range (min … max): 98.813 s … 108.429 s 10 runs Benchmark 3: npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno) Time (mean ± σ): 80.308 s ± 2.107 s [User: 74.331 s, System: 22.228 s] Range (min … max): 78.219 s … 84.277 s 10 runs Summary npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno) ran 1.24 ± 0.05 times faster than npm run test (branch = main) 1.29 ± 0.05 times faster than npm run test (branch = test/not-same-event-loop-concurrency) [49m1.944s] ╭─    ~/code/work/protocol.ai/ipfs/js-stores    284-chore-re…c-with-steno ?1 ╰─ ✔ ❯ hyperfine --parameter-list branch main,test/not-same-event-loop-concurrency,284-chore-replace-fast-write-atomic-with-steno --setup "git switch {branch} && npm run reset && npm i && npm run build && cd packages/blockstore-fs" "npm run test" Benchmark 1: npm run test (branch = main) Time (mean ± σ): 98.840 s ± 2.612 s [User: 68.486 s, System: 22.585 s] Range (min … max): 97.005 s … 104.396 s 10 runs Benchmark 2: npm run test (branch = test/not-same-event-loop-concurrency) Time (mean ± σ): 105.307 s ± 2.335 s [User: 72.442 s, System: 24.766 s] Range (min … max): 101.167 s … 109.007 s 10 runs Benchmark 3: npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno) Time (mean ± σ): 77.012 s ± 1.829 s [User: 74.442 s, System: 21.938 s] Range (min … max): 75.258 s … 80.825 s 10 runs Summary npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno) ran 1.28 ± 0.05 times faster than npm run test (branch = main) 1.37 ± 0.04 times faster than npm run test (branch = test/not-same-event-loop-concurrency) ```

SgtPooki · 2024-08-13T14:10:25Z

@achingbrain this PR originally had only tests but since #285 was merged, the changes are larger now. Are we good with merging this one?

SgtPooki · 2024-08-28T15:31:13Z

ping @achingbrain

## [blockstore-fs-v2.0.2](blockstore-fs-2.0.1...blockstore-fs-2.0.2) (2024-09-17) ### Tests * add separate-thread concurrency test ([#305](#305)) ([5e3114e](5e3114e)), closes [#285](#285) [#284](#284)

github-actions · 2024-09-17T12:37:04Z

🎉 This PR is included in version blockstore-fs-v2.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

## [datastore-fs-v10.0.2](datastore-fs-10.0.1...datastore-fs-10.0.2) (2024-09-17) ### Tests * add separate-thread concurrency test ([#305](#305)) ([5e3114e](5e3114e)), closes [#285](#285) [#284](#284)

github-actions · 2024-09-17T12:38:41Z

🎉 This PR is included in version datastore-fs-v10.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

SgtPooki added 2 commits April 19, 2024 10:28

tmp: testing concurrent writes with worker threads

4380b99

test: add separate-thread concurrency test

43726ef

SgtPooki commented Apr 19, 2024

View reviewed changes

SgtPooki mentioned this pull request Apr 19, 2024

fix: replace fast-atomic-write with steno #285

Merged

SgtPooki requested a review from achingbrain April 19, 2024 18:17

SgtPooki self-assigned this Apr 19, 2024

chore: re-throw errors from writer-worker

f435854

SgtPooki mentioned this pull request Apr 23, 2024

feat: sort open PRs by last updated time. SgtPooki/github-activity#2

Open

SgtPooki added 3 commits August 9, 2024 13:02

Merge branch 'main' into test/not-same-event-loop-concurrency

7691140

chore: fix lint error

f39b422

Merge branch 'main' into test/not-same-event-loop-concurrency

d6373c0

SgtPooki merged commit 5e3114e into main Sep 17, 2024
19 checks passed

SgtPooki deleted the test/not-same-event-loop-concurrency branch September 17, 2024 12:28

github-actions bot added the released label Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add separate-thread concurrency test #305

test: add separate-thread concurrency test #305

SgtPooki commented Apr 19, 2024

SgtPooki left a comment

SgtPooki Apr 19, 2024

SgtPooki Apr 19, 2024

SgtPooki Apr 19, 2024

SgtPooki Apr 19, 2024 •

edited

Loading

SgtPooki Apr 19, 2024 •

edited

Loading

SgtPooki Apr 22, 2024

SgtPooki commented Apr 20, 2024 •

edited

Loading

SgtPooki commented Apr 22, 2024

SgtPooki commented Aug 13, 2024

SgtPooki commented Aug 28, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

test: add separate-thread concurrency test #305

test: add separate-thread concurrency test #305

Conversation

SgtPooki commented Apr 19, 2024

SgtPooki left a comment

Choose a reason for hiding this comment

SgtPooki Apr 19, 2024

Choose a reason for hiding this comment

SgtPooki Apr 19, 2024

Choose a reason for hiding this comment

SgtPooki Apr 19, 2024

Choose a reason for hiding this comment

SgtPooki Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

SgtPooki Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

SgtPooki Apr 22, 2024

Choose a reason for hiding this comment

SgtPooki commented Apr 20, 2024 • edited Loading

SgtPooki commented Apr 22, 2024

SgtPooki commented Aug 13, 2024

SgtPooki commented Aug 28, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

SgtPooki Apr 19, 2024 •

edited

Loading

SgtPooki Apr 19, 2024 •

edited

Loading

SgtPooki commented Apr 20, 2024 •

edited

Loading