Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨Add implementation for JavaScript + Effection #73

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cowboyd
Copy link

@cowboyd cowboyd commented Aug 29, 2023

This is a JavaScript implementation using Effection Structured Concurrency Framework v3 Alpha running on Deno.

It currently stalls out on scenario three at about ~9500 concurrent requests.

$ deno task easyracer
Task easyracer deno run --allow-net --v8-flags=--max-old-space-size=8192 easyracer.ts
1: right
2: right
3: wrong (timeout of 10000ms exceeded)
4: right
5: right
6: right
7: right
8: right
9: right

@jamesward
Copy link
Owner

Nice timing! I'm just getting a vanilla JS client working:
https://github.com/jamesward/easyracer/tree/main/javascript-stdlib

Thanks for contributing this as I've wanted to get some TS clients in here! Any ideas on what is going wrong with scenario 3? Also, let's get the GitHub Action going with Testcontainers. You can reference my new JS client for how to do it.

@cowboyd
Copy link
Author

cowboyd commented Aug 30, 2023

@jamesward fantastic! I'm not sure what's going on with Scenario 3 other than it seems to be thrashing memory quite a bit. I've filed an issue on the deno repository to see if the Deno team has any ideas. In the meanwhile, I'll see how it runs on NodeJS.

In any case, I'll get the GithubAction going.

@cowboyd cowboyd force-pushed the main branch 5 times, most recently from 49ea74c to 106ad58 Compare August 30, 2023 19:28
@cowboyd cowboyd changed the title ✨Add implementation for JavaScript + Effection (Deno) ✨Add implementation for JavaScript + Effection Aug 30, 2023
@cowboyd
Copy link
Author

cowboyd commented Aug 30, 2023

@jamesward I've updated it to work both on Node and Deno, and I'm seeing the same sort of behavior on Node.JS. It makes me wonder if it's something to do with my operating system or not. I've noticed that if I use nvm to install a different node binary, the next time it runs it will open thousands of concurrent requests (although not quite getting to 10,000), but on subsequent times it will stall out at a few hundred.

@cowboyd
Copy link
Author

cowboyd commented Aug 30, 2023

Hmm... by following this answer https://stackoverflow.com/questions/55366733/increasing-limit-of-outgoing-tcp-connections-on-macos I can consistently get it approaching 10,000 requests every time, but it's still stalling out short.

@cowboyd
Copy link
Author

cowboyd commented Aug 30, 2023

I tried both Rust Futures and Tokio, and it was as smooth as butter, so it's definitely JS related.

@jamesward
Copy link
Owner

I'll give it a try on my Linux machine and report back.

@jamesward
Copy link
Owner

I ran this on Linux and it did make the 10k requests but failed the test for scenario 3 and then scenario 4:

$ deno task test
Task test deno test --allow-net --v8-flags=--max-old-space-size=8192
Check file:///home/jamesward/projects/easyracer/javascript-effection/easyracer.test.ts
running 1 test from ./easyracer.test.ts
easyracer ...
  scenario 1 ... ok (34ms)
  scenario 2 ... ok (1s)
  scenario 3 ... FAILED (16s)
  scenario 4 ... FAILED (1s)
  scenario 5 ... ok (1s)
  scenario 6 ... ok (1s)
  scenario 7 ... ok (3s)
  scenario 8 ... ok (14ms)
  scenario 9 ... ok (4s)
easyracer ... FAILED (due to 2 failed steps) (27s)

 ERRORS 

easyracer ... scenario 3 => https://deno.land/[email protected]/testing/_test_suite.ts:323:15
error: Leaking async ops:
  - 16 async operations to send a HTTP request were started in this test, but never completed. This is often caused by not awaiting the result of a `fetch` call.
To get more details where ops were leaked, run again with --trace-ops flag.

easyracer ... scenario 4 => https://deno.land/[email protected]/testing/_test_suite.ts:323:15
error: Leaking async ops:
  - 16 async operations to send a HTTP request were started before this test, but were completed during the test. Async operations should not complete in a test if they were not started in that test.
            This is often caused by not awaiting the result of a `fetch` call.
To get more details where ops were leaked, run again with --trace-ops flag.

 FAILURES 

easyracer ... scenario 3 => https://deno.land/[email protected]/testing/_test_suite.ts:323:15
easyracer ... scenario 4 => https://deno.land/[email protected]/testing/_test_suite.ts:323:15

FAILED | 0 passed (7 steps) | 1 failed (2 steps) (27s)

error: Test failed

I ran into some maybe similar issues with the javascript-stdlib one due to the close on fetch cancel taking a while, causing the fetch for the timeout'd request in 4 to timeout before the connection is made. Notes:
https://github.com/jamesward/easyracer/blob/main/javascript-stdlib/lib.js#L62-L66

Ultimately I was able to get it working there with some tricks.

@cowboyd
Copy link
Author

cowboyd commented Sep 1, 2023

@jamesward This is very interesting! By using http.request() directly in NodeJS I was able to get it working, because there is a callback you can get when the socket is closed. However, I was not able to do so in Deno. I'll see if I can't get it pushed up at some point.

In the mean time, I'll try and get CI working with testingcontainers following your example on stdlib

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants