feat: add workerIdleMemoryLimit to support worker recycling in the event of node >16.11.0 memory leaks #13056

phawxby · 2022-07-22T12:16:36Z

Summary

#11956 (comment)

Test plan

Numerous new test cases added except it's hard to test for the specific issue as the memory usage problem is extremely hard to replicate outside of very complicated applications

Ideally I would like to end-end test this but I have no idea how I would do that

phawxby · 2022-07-22T14:16:41Z

I'm going to try and write some more tests for this so even if approved don't merge yet

SimenB · 2022-07-24T00:15:22Z

packages/jest-types/src/Config.ts

@@ -478,6 +480,7 @@ export type ProjectConfig = {
  transformIgnorePatterns: Array<string>;
  watchPathIgnorePatterns: Array<string>;
  unmockedModulePathPatterns?: Array<string>;
+  workerIdleMemoryLimit?: number;


should be part of global config, not project

I can see the case for both. You may want to set a sensible global default perhaps running on a machine with known memory usage limitations but then configure specifically for projects where you may use less threads and allow more memory. In all honesty I could go either way, if you want it removing I can remove it.

SimenB · 2022-07-24T00:16:11Z

exciting! thanks for looking into it!

phawxby · 2022-07-25T13:49:33Z

@SimenB this should be better now. I've created edge case tests using actual workers (as opposed to mocked ones) to validate that they behave as expected.

docs/Configuration.md

phawxby · 2022-07-25T15:18:18Z

Are some of these tests expected to be failing?

SimenB · 2022-07-25T18:03:41Z

Are some of these tests expected to be failing?

There is some flake, but the failures now seems to be the new tests?

phawxby · 2022-07-25T18:17:47Z

Are some of these tests expected to be failing?

There is some flake, but the failures now seems to be the new tests?

Some of them seem to be a dependency bug. chalk/supports-color#135

phawxby · 2022-07-25T19:32:12Z

Are some of these tests expected to be failing?

There is some flake, but the failures now seems to be the new tests?

Some of them seem to be a dependency bug. chalk/supports-color#135

Actually I think I'm being dumb

phawxby · 2022-07-25T21:02:48Z

@SimenB do you have any good ideas why those 2 tests seem to be failing in CI? They work absolutely fine locally with the same commands.

SimenB

This new option is not supported in thread workers?

jest.config.ci.mjs

phawxby · 2022-07-27T19:33:06Z

This new option is not supported in thread workers?

I hadn't planned on it but I actually don't think it would be too hard to port over. I'll take a quick crack at it

phawxby · 2022-07-27T20:05:40Z

This new option is not supported in thread workers?

Actually, do we even know if this is necessary? This is primarily to deal with a memory leak in process workers. As far as I know there's currently no way to enable worker threads in jest via a config/cli option. It may be completely unnecessary in threaded workers.

SimenB · 2022-07-28T07:52:53Z

As far as I know there's currently no way to enable worker threads in jest via a config/cli option. It may be completely unnecessary in threaded workers.

There's not, but the same capability should be added to both types of workers. jest-worker is used outside of Jest itself by many projects, so there should be feature parity as far as possible. Jest will also switch at some point in the future

phawxby · 2022-08-01T13:34:27Z

I think this PR is generally done, I'm just not really sure what to do to resolve the flaky tests. They seem to work more often than not, but I know that's still not ideal.

Also the worker memory limit is specified in bytes, I wonder if that should be MB to make the number a bit easier to understand.

SimenB · 2022-08-01T14:25:59Z

Also the worker memory limit is specified in bytes, I wonder if that should be MB to make the number a bit easier to understand.

We could do shorthand? Bytes if no unit, but support e.g. 500m (converted to 1024 * 1024 * 500 in normalize)

phawxby · 2022-08-01T18:21:58Z

@SimenB done. I think I may have solved my flaky test problem too

SimenB

thanks! this looks good to me 👍

.github/workflows/test.yml

docs/Configuration.md

e2e/__tests__/workerForceExit.test.ts

packages/jest-config/src/__tests__/stringToBytes.test.ts

packages/jest-worker/src/workers/NodeThreadsWorker.ts

SimenB

thanks!

phawxby · 2022-08-06T19:37:11Z

@SimenB what's the release process/cycle for Jest?

SimenB · 2022-08-07T13:44:13Z

https://github.com/facebook/jest/releases/tag/v29.0.0-alpha.3

SimenB · 2022-08-07T13:44:29Z

@SimenB what's the release process/cycle for Jest?

Stable release next week

AndrewLeedham · 2022-08-08T10:18:28Z

Was trying this out this morning and ran into a config validation issue:

$ yarn jest --version

29.0.0-alpha.3

$ yarn jest

Validation Warning:

  Unknown option "workerIdleMemoryLimit" with value 0.2 was found.
  This is probably a typing mistake. Fixing it will remove this message.

  Configuration Documentation:
  https://jestjs.io/docs/configuration

The jest.config.js has workerIdleMemoryLimit: 0.2, at the root of the config object.

Seems to me like workerIdleMemoryLimit needs to be added to https://github.com/facebook/jest/blob/main/packages/jest-config/src/ValidConfig.ts ?

github-actions · 2022-09-08T00:11:07Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

feat: process recycling

3e4c26b

facebook-github-bot added the cla signed label Jul 22, 2022

phawxby added 2 commits July 22, 2022 13:20

chore: changelog

833e429

chore: remove temp files

7da8127

phawxby marked this pull request as ready for review July 22, 2022 13:45

phawxby changed the title ~~feat: process recycling~~ feat: add workerIdleMemoryLimit to support worker recycling in the event of node >16.11.0 memory leaks Jul 22, 2022

SimenB reviewed Jul 24, 2022

View changes

test: more complete functional testing

40a23cf

mrazauskas reviewed Jul 25, 2022

View changes

docs/Configuration.md Show resolved Hide resolved

docs/Configuration.md Outdated Show resolved Hide resolved

phawxby added 2 commits July 25, 2022 15:25

docs: update docs for PR review

1961d65

chore: try this test config

d7f292f

phawxby added 3 commits July 25, 2022 17:02

chore: linting

8bfcd74

feat: add improved error checking and promise handling

fd1be6f

chore: add facebook header

233477e

phawxby added 5 commits July 25, 2022 20:39

fix: use os spy rather than mock to retain platform functions

19fb71c

fix: spying of totalmem

e2a853d

chore: add some logging to help debugging

ab6554d

chore: more debugging

207ae05

chore: more debugging

c4d2389

phawxby added 3 commits July 25, 2022 22:15

chore: check files exist

0f5c7bb

chore: try as a specific js import

d35e4b9

chore: try this

daee499

SimenB reviewed Jul 27, 2022

View changes

jest.config.ci.mjs Outdated Show resolved Hide resolved

chore: revert changes to jest config

ba00b26

phawxby added 2 commits July 28, 2022 15:11

feat: port idle usage to thread workers

019c5d4

chore: docs

7239650

phawxby requested a review from SimenB July 28, 2022 14:41

chore: linting

fdb4a66

phawxby added 3 commits August 1, 2022 17:31

feat: add shorthand idle limit parsing

3e3be99

docs: copyright header

9f9dff1

chore: improve test flake

43f1054

chore: fix typing

791b7a5

phawxby mentioned this pull request Aug 2, 2022

[Bug]: Memory consumption issues on Node JS 16.11.0+ #11956

Open

SimenB mentioned this pull request Aug 3, 2022

[Feature]: Add a setting to limit worker memory utilization #12893

Closed

SimenB approved these changes Aug 4, 2022

View changes

chore: pr feedback

f3db1cc

phawxby requested a review from SimenB August 4, 2022 21:47

SimenB approved these changes Aug 5, 2022

View changes

SimenB merged commit 37200eb into facebook:main Aug 5, 2022
64 checks passed

AndrewLeedham mentioned this pull request Aug 8, 2022

fix: add workerIdleMemoryLimit to ValidConfig #13105

Merged

github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2022

feat: add workerIdleMemoryLimit to support worker recycling in the event of node >16.11.0 memory leaks #13056

feat: add workerIdleMemoryLimit to support worker recycling in the event of node >16.11.0 memory leaks #13056

phawxby commented Jul 22, 2022 •

edited

phawxby commented Jul 22, 2022

SimenB Jul 24, 2022

phawxby Jul 25, 2022

SimenB commented Jul 24, 2022

phawxby commented Jul 25, 2022

phawxby commented Jul 25, 2022

SimenB commented Jul 25, 2022

phawxby commented Jul 25, 2022

phawxby commented Jul 25, 2022

phawxby commented Jul 25, 2022

SimenB left a comment

phawxby commented Jul 27, 2022

phawxby commented Jul 27, 2022

SimenB commented Jul 28, 2022

phawxby commented Aug 1, 2022

SimenB commented Aug 1, 2022 •

edited

phawxby commented Aug 1, 2022

SimenB left a comment

SimenB left a comment

phawxby commented Aug 6, 2022

SimenB commented Aug 7, 2022

SimenB commented Aug 7, 2022

AndrewLeedham commented Aug 8, 2022

github-actions bot commented Sep 8, 2022

feat: add workerIdleMemoryLimit to support worker recycling in the event of node >16.11.0 memory leaks #13056

feat: add workerIdleMemoryLimit to support worker recycling in the event of node >16.11.0 memory leaks #13056

Conversation

phawxby commented Jul 22, 2022 • edited

Summary

Test plan

phawxby commented Jul 22, 2022

SimenB Jul 24, 2022

Choose a reason for hiding this comment

phawxby Jul 25, 2022

Choose a reason for hiding this comment

SimenB commented Jul 24, 2022

phawxby commented Jul 25, 2022

phawxby commented Jul 25, 2022

SimenB commented Jul 25, 2022

phawxby commented Jul 25, 2022

phawxby commented Jul 25, 2022

phawxby commented Jul 25, 2022

SimenB left a comment

Choose a reason for hiding this comment

phawxby commented Jul 27, 2022

phawxby commented Jul 27, 2022

SimenB commented Jul 28, 2022

phawxby commented Aug 1, 2022

SimenB commented Aug 1, 2022 • edited

phawxby commented Aug 1, 2022

SimenB left a comment

Choose a reason for hiding this comment

SimenB left a comment

Choose a reason for hiding this comment

phawxby commented Aug 6, 2022

SimenB commented Aug 7, 2022

SimenB commented Aug 7, 2022

AndrewLeedham commented Aug 8, 2022

github-actions bot commented Sep 8, 2022

phawxby commented Jul 22, 2022 •

edited

SimenB commented Aug 1, 2022 •

edited