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

Fix sample collection when using worker threads #309

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jasnell
Copy link
Contributor

@jasnell jasnell commented Apr 2, 2020

When sampler.js is running in a worker thread, redirect processstat to a different file and suppress the systeminfo file.

@goto-bus-stop .. in my local environment I'm seeing a failure in certain tests that do not appear to be directly related but might be...

 PASS  test/analysis-cpu.test.js 40 OK 1s
 PASS  test/analysis-delay.test.js 8 OK 45.155ms
 PASS  test/analysis-guess-interval.test.js 16 OK 56.536ms
 PASS  test/analysis-handles.test.js 12 OK 50.897ms
 PASS  test/analysis-issue-category.test.js 20 OK 27.577ms
 PASS  test/analysis-memory.test.js 11 OK 93.541ms
 PASS  test/analysis.test.js 10 OK 1s
 PASS  test/cmd-collect-analysing-worker.test.js 4 OK 587.209ms
 PASS  test/cmd-collect-analysing.test.js 4 OK 440.233ms
 PASS  test/cmd-collect-detect-port.test.js 6 OK 600.236ms
test/cmd-collect-exit.test.js 2> process exited with exit code 1
 SKIP  test/cmd-collect-exit.test.js
 ~ cmd - collect - external SIGINT is relayed > Skip test as we cannot easily send SIGINT on Windows

 SKIP  test/cmd-collect-exit.test.js
 ~ cmd - collect - SIGKILL causes error > Skip test as SIGKILL also sends exit code 1 on Windows

 SKIP  test/cmd-collect-exit.test.js 2 skip of 3 392.024ms
 ~ cmd - collect - external SIGINT is relayed > Skip test as we cannot easily send SIGINT on Windows
 ~ cmd - collect - SIGKILL causes error > Skip test as SIGKILL also sends exit code 1 on Windows

 PASS  test/cmd-collect-node-options-env.test.js 2 OK 2s
 PASS  test/cmd-collect-node-path-env.test.js 2 OK 2s
 PASS  test/cmd-collect-sample-interval.test.js 2 OK 1s
 PASS  test/cmd-collect.test.js 11 OK 2s
 PASS  test/cmd-no-cluster.test.js 3 OK 3s
 FAIL  test/cmd-visualize.test.js 1230 OK 50s
  command: C:\Users\jasne\AppData\Local\nvs\node\13.11.0\x64\node.exe
  args:
    - -r
    - C:\Users\jasne\Projects\clinic\node-clinic-doctor\node_modules\esm\esm.js
    - test/cmd-visualize.test.js
  exitCode: null
  signal: SIGTERM

 PASS  test/collect-get-logging-paths.test.js 8 OK 84.679ms
 PASS  test/collect-process-stat.test.js 10 OK 496.138ms
 PASS  test/collect-system-info.test.js 5 OK 27.585ms
 PASS  test/format-process-stat.test.js 9 OK 49.158ms
 PASS  test/format-system-info.test.js 1 OK 36.129ms
 PASS  test/format-trace-event.test.js 2 OK 44.636ms
 PASS  test/lib-destroyable-stream.test.js 20 OK 46.163ms
 PASS  test/recommendation.test.js 62 OK 224.213ms

@jasnell
Copy link
Contributor Author

jasnell commented Apr 3, 2020

10.x tests failed because worker_threads is still behind a flag on 10.x

Not sure why the 13.x test failed. Appears unrelated.

@jasnell jasnell changed the base branch from master to main August 21, 2020 19:00
@esmorodin
Copy link

Confirm, that this fix works, because otherwise I faced this error: clinicjs/node-clinic#414.
Node version: bash-3.2$ node -v v18.12.0
FYI @jasnell

@RafaelGSS
Copy link
Contributor

RafaelGSS commented Oct 28, 2022

This probably needs a rebase. Once the test passes, we can merge it.

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.

3 participants