[securitySolution] Jest memory leaks #117255
Comments
tylersmalley
added
bug
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
@paul-tavares - can you take a look at the above from our team's perspective? A few files from OLM team show up here. |
|
@kevinlog Sorry I missed this. I have added this to our board and also added my name to the assignee list. I will take a look |
|
@paul-tavares , @rylnd , @tylersmalley if this helps I spent the afternoon reading through reasons why it can be a memory leak. A lot of things. However, I think some test files leak and some do not. I made this small script utility which copies a targeted test file multiple times and then executes that one particular test multiple times in order to see if there is leak. It looks to allow me to target specific files after narrowing down to a particular directory: #!/bin/sh
# Set your kibana home here
KIBANA_HOME=~/projects/kibana
MAX_MEMORY=800
# Useage if no arguments
if [ $# -eq 0 ]; then
echo "Usage is ./copy_tests.sh <test name>"
exit 1
fi
# Check if file exists or not
if [[ ! -f $1 ]]; then
echo "File does not exist"
exit 1
fi
# Gets the directory of your first argument
DIR="$(dirname "${1}")"
# Loops max times and copy the single test in place
for i in {1..100}
do
cp $1 $DIR/script_copy_test_$i.test.ts
done
pushd $KIBANA_HOME;
node --max-old-space-size=$MAX_MEMORY --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage --detectOpenHandles --no-cache --config x-pack/plugins/security_solution/jest.config.js $DIR/script_copy_test_*.ts;
popd;
# Remove all the copied scripts
rm $DIR/script_copy_test_*.test.tsFor example I think a lot of tests in the signals folder has leaks like ./copy_tests.sh ~/projects/kibana/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_event_type_signal.test.tsBut if I run this instead against ./copy_tests.sh ~/projects/kibana/x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_migration.test.tsYou can add |
paul-tavares
added
OLM Sprint
Team:Defend Workflows
|
Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt) |
|
FYI - @academo was already looking into some issues with Jest last week, and after speaking with him, he will continue no this issue and follow through on the memory leaks |
|
I updated my script above with this gist one now. I isolated one memory leak coming from alerting. Take this simple gist file: And you can put that file in For where we import from the top level alerting plugin, we can easily just change that code to be: import type {
AlertInstanceContext,
AlertInstanceState,
AlertServices,
} from '../../../../../alerting/server';
// Import this not from above or you will get memory leaks from Jest tests
import { parseDuration } from '../../../../../alerting/common/parse_duration';Then we get type erasure since we pulling Performance is also hindered and directly related to the sheer number of modules we are pulling in with our tests. Our tests will do better memory and performance wise if we minimize the imports. But...I can't say, "why" this is leaking memory from the alerting side and if you look at even well behaved memory heaps we are pulling in ReactJS and other modules for even server side tests into memory and we do it from 1 to 10 times for even non-leaking tests. I even did the garbage collect tricks and lowered my memory to ensure I was seeing for sure that in a lot of cases we are loading a lot of memory and reseting that memory between test runs which is a big performance impact. For example with the Here is well behaved tests with Stable when it first starts: Stable after a while of running: Here when First heap snapshot, already large: After a while, heap snapshot grows even though I call the garbage collection (modules cannot be garbage collected): |
|
I just happened to see this issue - so thought I'd mention a leak we just noticed. We noticed it doing some alerting stress testing in the 7.16.0 BC's - #116636 . Caused by a new feachur in node v16 in |
|
What happens if you don't use I think that combined with all the This doesn't seem like something alerting has done - explicitly or implicitly - to cause this, but obviously we want to know if it is. Is there something specific we should be looking at? Maybe we need to organize our imports better to allow this kind of usage? For instance, is it that accessing the |
I would look at In all places you can add Would be helpful to reduce and maybe even remove issues if not at least we can tell looking at a file where it is pulling imports from another vs. it is just using types.
Yes, a lot less modules and the memory leak doesn't show up.
For debugging memory issues I think everyone recommends running it in band. You can remove the Even starting up it consumes a lot of memory. |
|
Second one I think I found somewhere when importing I think I can patch import { actionsClientMock } from './mocks';I am going to import like so directly: import { actionsClientMock } from './actions_client.mock';And early results shows that doesn't leak. |
|
@pmuellr, |
|
I was checking this test in particular Doing some kind of bisect by commenting and enabling parts of the tests here and there I found that the memory leak only happens when this imported method is used: I have to comment or remove the code from the test (skip is not enough) to stop the leak. Also is this specific line that is causing the leak on this specific test. I remove all other lines and left the test with only this and it happened. Thinking that this specific mock code was creating the memory leak I went to the source and replace it by returning a simple object instead of the regular code but it kept failing. I then replace the whole file to only export this method that merely returned an object and there was no memory leak. Doing now more bisect here and there in that specific method I found the leak only occurred when you import this module that module itself is an object of strings (not an enum) |
|
It's hard to see where but if you're tracing through you eventually find the same curious case where if your test through an import either directly or transitive imports a function through the pattern: x-pack/plugins/${PLUGIN_NAME}/server/index.ts It will do two things:
Here is a small test case which shows us leaking memory when we import from: x-pack/plugins/fleet/server/index.ts // Put this file at "x-pack/plugins/fleet/server" and then modify "copy_tests.sh"
// to have the setting "KIBANA_PROJECT=x-pack/plugins/fleet/jest.config.js"
// And then run it:
// ./copy_tests.sh ~/projects/kibana/x-pack/plugins/fleet/server/test_sample.test.ts
// And you will see the memory leak unless you use the import outside of the pattern
// "x-pack/plugins/fleet/server"
// Any functions we import directly from ${PLUGIN_NAME}/server seems to start the memory leak.
// This will be a memory leak if we import from "x-pack/plugins/fleet/server"
// Comment this out and uncomment the line underneath it to remove the memory leak
import { relativeDownloadUrlFromArtifact } from '.';
// This will not be a memory leak if we import from within the mappings folder
// import { relativeDownloadUrlFromArtifact } from './services/artifacts/mappings';
describe('memory leak', () => {
test('it has a memory leak by just iterating over an import', () => {
// You just have to ensure you're touching the import, not actually invoke it to see the leak
Object.keys(relativeDownloadUrlFromArtifact);
});
});Notice that I'm not running the function, I'm just touching it to ensure jest knows to load it. You could call the function and it's the same effect but this lets everyone know it's something with the imports and not specific code. You can change your jest settings and add cleanup routines as well and expose the garbage collector from NodeJS and run that like I did earlier but it leaks the same. Once you import the file directly from the underlying |
|
Can confirm I was able to reproduce the memory leak following Frank's instructions. I captured a few heap snapshots and spent awhile digging through them, but was not able to find anything conclusive (though my analysis was certainly not exhaustive). In particular I am interested in Frank's theory about whether there's something going on pulling in a lot of things from |
|
@lukeelmers I did code bisecting yesterday with @FrankHassanabad's script and narrowed down the problem to two pieces of code: file1: const root = new Root(rawConfigService, env, onRootShutdown);file2: export const config = {
elasticsearch: {
schema: elasticsearchConfigSchema,
},
// this needs commenting
//logging: {
// appenders: appendersSchema as Type<AppenderConfigType>,
//},
};removing those both parts of code stops all memory leaking, removing only one of the parts has no effect. I didn't go further on investigating the logging or Root objects but it might give you a clue where to start looking. Note that merely commenting the code is enough even if it creates types or syntax errors. |
|
Thanks @academo! This is super helpful. I'll go ahead and label this issue for the core team to investigate further. We'll keep you posted. |
lukeelmers
added
the
Team:Core
…600 megs (elastic#118734) ## Summary This addresses parts of elastic#117255 By introducing top level mocks for: * `core/server/index.ts` * `task_manager/server/index.ts` * `alerting/server/index.ts` * `actions/server/index.ts` These top level mocks add the few required functions we use sparingly and adds them from the "restricted zones" to avoid giant typescript imports from happening from the server side which also pulls in the memory leaks. ```ts moduleNameMapper: { 'core/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/core.mock.ts', 'task_manager/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/task_manager.mock.ts', 'alerting/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/alert.mock.ts', 'actions/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/action.mock.ts', }, ``` For testing this you can now run: ```sh node --max-old-space-size=600 --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage --detectOpenHandles --no-cache --config x-pack/plugins/security_solution/jest.config.dev.js x-pack/plugins/security_solution/server ``` And the server side tests will be able to complete in less than 600 megs of memory. The memory leaks and memory consumption issues are mitigated through the layers but this doesn't guarantee that in the future these won't show up again. The root of the issue(s) with the memory leaks from `core/server` aren't addressed here as those are separate concerns at this point but this at least mitigates the amount of leakage from our side for now. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
…600 megs (#118734) (#118930) ## Summary This addresses parts of #117255 By introducing top level mocks for: * `core/server/index.ts` * `task_manager/server/index.ts` * `alerting/server/index.ts` * `actions/server/index.ts` These top level mocks add the few required functions we use sparingly and adds them from the "restricted zones" to avoid giant typescript imports from happening from the server side which also pulls in the memory leaks. ```ts moduleNameMapper: { 'core/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/core.mock.ts', 'task_manager/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/task_manager.mock.ts', 'alerting/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/alert.mock.ts', 'actions/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/action.mock.ts', }, ``` For testing this you can now run: ```sh node --max-old-space-size=600 --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage --detectOpenHandles --no-cache --config x-pack/plugins/security_solution/jest.config.dev.js x-pack/plugins/security_solution/server ``` And the server side tests will be able to complete in less than 600 megs of memory. The memory leaks and memory consumption issues are mitigated through the layers but this doesn't guarantee that in the future these won't show up again. The root of the issue(s) with the memory leaks from `core/server` aren't addressed here as those are separate concerns at this point but this at least mitigates the amount of leakage from our side for now. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Frank Hassanabad <[email protected]>
|
@FrankHassanabad I see you merged #118734. Was that blocking you for something? As all test suites are impacted, I wonder if that kind of local patching / workaround is really a good idea? Also FWIW, this issue is planned for our next sprint (which starts next week) |
…600 megs (elastic#118734) ## Summary This addresses parts of elastic#117255 By introducing top level mocks for: * `core/server/index.ts` * `task_manager/server/index.ts` * `alerting/server/index.ts` * `actions/server/index.ts` These top level mocks add the few required functions we use sparingly and adds them from the "restricted zones" to avoid giant typescript imports from happening from the server side which also pulls in the memory leaks. ```ts moduleNameMapper: { 'core/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/core.mock.ts', 'task_manager/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/task_manager.mock.ts', 'alerting/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/alert.mock.ts', 'actions/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/action.mock.ts', }, ``` For testing this you can now run: ```sh node --max-old-space-size=600 --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage --detectOpenHandles --no-cache --config x-pack/plugins/security_solution/jest.config.dev.js x-pack/plugins/security_solution/server ``` And the server side tests will be able to complete in less than 600 megs of memory. The memory leaks and memory consumption issues are mitigated through the layers but this doesn't guarantee that in the future these won't show up again. The root of the issue(s) with the memory leaks from `core/server` aren't addressed here as those are separate concerns at this point but this at least mitigates the amount of leakage from our side for now. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
|
Gonna take some time to take a second look on this. Thanks @FrankHassanabad for the script btw, helps a lot. A remark though, In the provided sample test (the one importing Even an empty test, without any import, does trigger a heap increase 10mb increase on each file seems like enormous for just an empty test file? |
|
Okay nevermind, that's the time jest start all the parallel runner in the process. Once all runners are up, the memory stabilize when running the empty test. |
|
Spent a few hours, didn't really find anything relevant, but here's some observations: Memory consumption on an empty testOkay, so... First thing I don't get is that the memory consumption is different when attached to the debugger or not. (when using, or not, Using a plain empty test: // no imports at all
describe("I've no idea what I'm doing", () => {
beforeEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
jest.restoreAllMocks();
jest.resetModules();
jest.clearAllMocks();
});
afterEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
jest.restoreAllMocks();
jest.resetModules();
});
test('parseDuration', () => {
if (global.gc) {
// console.log('I am able to GC...');
global.gc();
} else {
// console.log('I cannot GC...');
}
expect(true).toEqual(true);
});
});Running the script with Now, running the script with Memory leak and core importNow, the second thing is, I can't reproduce the memory not leaking when commenting the lines in core as specified in #117255 (comment) With this test // `x-pack/plugins/alerting/server/sample_test.ts`
import { parseDuration } from '.';
describe('memory leak as soon as we pull in and use "parseDuration" from "alerting', () => {
beforeEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
jest.restoreAllMocks();
jest.resetModules();
jest.clearAllMocks();
});
afterEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
jest.restoreAllMocks();
jest.resetModules();
});
test('parseDuration', () => {
if (global.gc) {
// console.log('I am able to GC...');
global.gc();
} else {
// console.log('I cannot GC...');
}
parseDuration('5s');
expect(true).toEqual(true);
});
});and these patches (the ones suggested to be solving the leak in #117255 (comment)): Index: src/core/server/bootstrap.ts
===================================================================
diff --git a/src/core/server/bootstrap.ts b/src/core/server/bootstrap.ts
--- a/src/core/server/bootstrap.ts (revision 14ed0cb89952fd6ff904f723b2e0a435961a5aa2)
+++ b/src/core/server/bootstrap.ts (date 1637839868421)
@@ -8,7 +8,7 @@
import chalk from 'chalk';
import { CliArgs, Env, RawConfigService } from './config';
-import { Root } from './root';
+// import { Root } from './root';
import { CriticalError } from './errors';
interface BootstrapArgs {
@@ -44,7 +44,7 @@
const rawConfigService = new RawConfigService(env.configs, applyConfigOverrides);
rawConfigService.loadConfig();
- const root = new Root(rawConfigService, env, onRootShutdown);
+ // const root = new Root(rawConfigService, env, onRootShutdown);
process.on('SIGHUP', () => reloadConfiguration());Index: src/core/server/index.ts
===================================================================
diff --git a/src/core/server/index.ts b/src/core/server/index.ts
--- a/src/core/server/index.ts (revision 14ed0cb89952fd6ff904f723b2e0a435961a5aa2)
+++ b/src/core/server/index.ts (date 1637839900052)
@@ -54,7 +54,7 @@
import { CapabilitiesSetup, CapabilitiesStart } from './capabilities';
import { MetricsServiceSetup, MetricsServiceStart } from './metrics';
import { StatusServiceSetup } from './status';
-import { AppenderConfigType, appendersSchema, LoggingServiceSetup } from './logging';
+// import { AppenderConfigType, appendersSchema, LoggingServiceSetup } from './logging';
import { CoreUsageDataStart, CoreUsageDataSetup } from './core_usage_data';
import { I18nServiceSetup } from './i18n';
import { DeprecationsServiceSetup, DeprecationsClient } from './deprecations';
@@ -579,6 +579,6 @@
schema: elasticsearchConfigSchema,
},
logging: {
- appenders: appendersSchema as Type<AppenderConfigType>,
+ // appenders: appendersSchema as Type<AppenderConfigType>,
},
};With Pierres-MBP-2:kibana pgayvallet$ ./memory_leak.sh x-pack/plugins/alerting/server/sample_test.ts
~/DEV/workspaces/elastic/kibana ~/DEV/workspaces/elastic/kibana
PASS x-pack/plugins/alerting/server/script_copy_test_75.test.ts (37.586 s, 175 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_17.test.ts (255 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_65.test.ts (310 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_91.test.ts (371 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_81.test.ts (431 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_28.test.ts (492 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_38.test.ts (552 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_5.test.ts (613 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_53.test.ts (673 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_43.test.ts (734 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_88.test.ts (796 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_98.test.ts (856 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_31.test.ts (916 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_21.test.ts (977 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_42.test.ts (1037 MB heap size)
PASS x-pack/plugins/alerting/server/script_copy_test_52.test.ts (1097 MB heap size)With Even when being very aggressive with the cuts from // src/core/server/index.ts
export { DEFAULT_APP_CATEGORIES, APP_WRAPPER_CLASS } from '../utils';
export { ServiceStatusLevels } from './status/types';
export const config = {};
// yup, that's allI observe the same leak (here with But I do confirm that using the absolute import for -- import { parseDuration } from '.';
++ import { parseDuration } from '../common/parse_duration';give the following output Importing directly from
|
|
Okay. I've been doing all my tests with Now, I went back to my tests (all tests includes a
FWIW, empty export const ServiceStatusLevels = { available: '', degraded: '', unavailable: '', critical: '' };
export const config = {};
export const APP_WRAPPER_CLASS = '';
export const DEFAULT_APP_CATEGORIES = {};So unless someone can come with a scenario where importing indirectly from core would cause a leak when importing directly doesn't, plus the fact that the leak occurs when |
|
@pgayvallet thanks for taking the time and running the tests. did you check the part I reported about the Root class and the logging? |
…600 megs (#118734) ## Summary This addresses parts of #117255 By introducing top level mocks for: * `core/server/index.ts` * `task_manager/server/index.ts` * `alerting/server/index.ts` * `actions/server/index.ts` These top level mocks add the few required functions we use sparingly and adds them from the "restricted zones" to avoid giant typescript imports from happening from the server side which also pulls in the memory leaks. ```ts moduleNameMapper: { 'core/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/core.mock.ts', 'task_manager/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/task_manager.mock.ts', 'alerting/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/alert.mock.ts', 'actions/server$': '<rootDir>/x-pack/plugins/security_solution/server/__mocks__/action.mock.ts', }, ``` For testing this you can now run: ```sh node --max-old-space-size=600 --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage --detectOpenHandles --no-cache --config x-pack/plugins/security_solution/jest.config.dev.js x-pack/plugins/security_solution/server ``` And the server side tests will be able to complete in less than 600 megs of memory. The memory leaks and memory consumption issues are mitigated through the layers but this doesn't guarantee that in the future these won't show up again. The root of the issue(s) with the memory leaks from `core/server` aren't addressed here as those are separate concerns at this point but this at least mitigates the amount of leakage from our side for now. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
@pgayvallet That's only for tests within That PR does reduce the leaks. I can remove it once the leaks are fixed. One thing of note is that the performance of the tests improved too since we cut away those parts (as an aside). |
|
++ to @pgayvallet running our tests... 🙏 thank you for looking at this.
@pgayvallet , I think maybe a big pro of what I did with #118734 is that I can test when a particular plugin such as For example, in this draft PR here I removed and then if I run this command: node --max-old-space-size=1000 ./node_modules/.bin/jest --runInBand --logHeapUsage --no-cache --config x-pack/plugins/security_solution/jest.config.dev.js x-pack/plugins/security_solution/serverWith |
|
In addition to my advice above with the draft PR, you can re-use the and run it like so: ./copy_tests.sh ~/projects/kibana/src/core/server/server.test.tsI see it blow up with an OOM after ~32 iterations which seems to indicate at least that test which is isolated to You can change the iterations and the memory size to see that test appears to be leaking memory. For example I changed the loop to be an iteration cycle of |
Yea, as described in #117255 (comment), I performed a test in the I just checked @FrankHassanabad's scenario regarding around the 43th iteration. The leak is so small that you can't see it just by looking at the heap size variation between the runs though, which gonna make it incredibly fun to progress further, as it takes a LOT of time to observe the presence of not of the leak. Now, given that
and
and
my conclusion is that the source of the leak is coming from a import performed both from the import tree of @academo do you remember which test you used to have the changes you described in #117255 (comment) get rid of the leak? I would love to have a reproduction scenario with a (single) test file where the leak (and non-leak) is more significant per run to avoid having to wait 10mins between each bisection. |
|
@pgayvallet I used a dummy test inside fleet. Literally a new file with a describe, one test that only imported the server/core and then |
|
After thinking a bit about the From my understanding of https://chanind.github.io/javascript/2019/10/12/jest-tests-memory-leak.html, a leak because of patching native modules would be way more significant. The example they're using to demonstrate the leak is quite simple, and the dependency tree is way smaller than what we can have by just what I tried to attach the debugger to inspect a memory dump while running to see if I could find anything, but as observed in #117255 (comment), if the suite completes without Just to have a track of it, I installed PID 51656 received SIGSEGV for address: 0x0
0 segfault-handler.node 0x0000000113996fb0 _ZL16segfault_handleriP9__siginfoPv + 304
1 libsystem_platform.dylib 0x00007fff20377d7d _sigtramp + 29
2 ??? 0x0000000000000004 0x0 + 4
3 node 0x000000010eff3020 _ZN2v88internal13GlobalHandles4Node31PostGarbageCollectionProcessingEPNS0_7IsolateE + 128
4 node 0x000000010eff3aa7 _ZN2v88internal13GlobalHandles31PostGarbageCollectionProcessingENS0_16GarbageCollectorENS_15GCCallbackFlagsE + 199
5 node 0x000000010f03a8f5 _ZN2v88internal4Heap14CollectGarbageENS0_15AllocationSpaceENS0_23GarbageCollectionReasonENS_15GCCallbackFlagsE + 2453
6 node 0x000000010f03bf9f _ZN2v88internal4Heap24PreciseCollectAllGarbageEiNS0_23GarbageCollectionReasonENS_15GCCallbackFlagsE + 95
7 node 0x000000010ef03dd9 _ZN2v88internal25FunctionCallbackArguments4CallENS0_15CallHandlerInfoE + 265
8 node 0x000000010ef038a6 _ZN2v88internal12_GLOBAL__N_119HandleApiCallHelperILb0EEENS0_11MaybeHandleINS0_6ObjectEEEPNS0_7IsolateENS0_6HandleINS0_10HeapObjectEEESA_NS8_INS0_20FunctionTemplateInfoEEENS8_IS4_EENS0_16BuiltinArgumentsE + 550
9 node 0x000000010ef0301f _ZN2v88internal21Builtin_HandleApiCallEiPmPNS0_7IsolateE + 255
10 node 0x000000010f773eb9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit + 57
11 node 0x000000010f7068aa Builtins_InterpreterEntryTrampoline + 202
./memory_leak.sh: line 54: 51656 Segmentation fault: 11
I double-checked just to be sure, but I can confirm I do not get to the same observations With this test ran from The memory leaks (~45mb per run) even with or without this patch applied to go further, I also tried to totally mock by adding this to moduleNameMapper: {
'core/server$': '<rootDir>/x-pack/plugins/alerting/core.mock.ts',
}with export const bootstrap = 'foo';
export const ServiceStatusLevels = { available: '', degraded: '', unavailable: '', critical: '' };
export const config = {};
export const APP_WRAPPER_CLASS = '';
export const DEFAULT_APP_CATEGORIES = {};and this the test file adapted just to be sure that the module name mapper is used: import { parseDuration } from '.';
import { bootstrap } from '../../../../src/core/server';
describe('memory leak as soon as we pull in and use "parseDuration" from "alerting', () => {
test('parseDuration', () => {
expect(bootstrap).toEqual('foo');
Object.keys(parseDuration);
});
});and I observe the same ~45mb per run leak. Now, this is where it gets interesting. If I also stub all moduleNameMapper: {
'core/server$': '<rootDir>/x-pack/plugins/alerting/core.mock.ts',
'actions/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'spaces/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'security/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'licensing/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'task_manager/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'event_log/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'encrypted_saved_objects/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'features/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'usage_collection/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
'kibana_utils/server$': '<rootDir>/x-pack/plugins/alerting/dep.mock.ts',
},The memory is stable. So this really makes me think that the 1mb memory increase observed in |
|
So, I started commenting one by one the stubs in FWIW, I used 250MB max memory and 100 runs, and assumed there was no leak if the runs completed successfully. NOT LEAKING when using real module:
LEAKING when using real module:
Even with all other dependencies mocked, the leak is obvious and 'clean' (approx 15/20mb per run increase). Just to be sure, I also did a test with all dependencies using the real implementation except for All this means the cause is somewhere within cc @elastic/kibana-alerting-services this doesn't mean this is coming directly from |
|
So, All the following tests were performed in the same 'sandbox' as other tests (with First thing I did was to see if importing Then, I used bisections to try to get closer to the source. And here's something interesting: I isolated a reproduction scenario with only two imports: import { getAuthorizationModeBySource } from '../../actions/server/authorization/get_authorization_mode_by_source';
import { ensureSufficientLicense } from '../../actions/server/lib/ensure_sufficient_license';meaning that this reproduces the leak: import { getAuthorizationModeBySource } from '../../actions/server/authorization/get_authorization_mode_by_source';
import { ensureSufficientLicense } from '../../actions/server/lib/ensure_sufficient_license';
describe('memory leak are hard', () => {
test('this leaks', () => {
Object.keys(getAuthorizationModeBySource);
Object.keys(ensureSufficientLicense);
});
});However, the problem seems to be caused by some interactions between the two files (or their nested import trees), because using only one import, or the other, does not reproduce the leak. This does not leak // import { getAuthorizationModeBySource } from '../../actions/server/authorization/get_authorization_mode_by_source';
import { ensureSufficientLicense } from '../../actions/server/lib/ensure_sufficient_license';
describe('memory leak are hard', () => {
test('this does not leak', () => {
// Object.keys(getAuthorizationModeBySource);
Object.keys(ensureSufficientLicense);
});
});This does not either import { getAuthorizationModeBySource } from '../../actions/server/authorization/get_authorization_mode_by_source';
// import { ensureSufficientLicense } from '../../actions/server/lib/ensure_sufficient_license';
describe('memory leak are hard', () => {
test('this does not leak', () => {
Object.keys(getAuthorizationModeBySource);
// Object.keys(ensureSufficientLicense);
});
});Not 100% sure how to progress further. I guess I will try to comment imports in these files see what it does. Also note that given that these two imports are causing the leak, there may be other combination(S) of imports from |
Did you check whether there are circular imports between the files? |
|
I isolated the imports in the two files that are causing this conflict-ish situation In import { ActionExecutionSource, isSavedObjectExecutionSource } from '../lib';in import { ServerLogActionTypeId, IndexActionTypeId } from '../builtin_action_types';Having only these lines in each file causes the leak. Commenting one (or both) removes the leak. I then tried to reproduce the leak by importing these two dependencies directly, but this does NOT cause a leak: import '../../actions/server/builtin_action_types';
import '../../actions/server/lib';
describe('memory leak are hard', () => {
test('because we can', () => {
expect('leak').toBeTruthy();
});
});At this point I agree with @mshustov, this looks like a circular dependency between I apologize about the very verbose reporting by the way, but I want to keep track of the whole thinking in case we'll need to eventually do this again elsewhere. |
|
So... I tried identifying a circular dependency passing though both those files, but unfortunately without any success
To be honest, I had issues with both scripts.
Even after manually changing
Couldn't find a way to have him ignoring type imports, but even when following them, the script reports a lot less circular import results than madge, so... Also, in theory, circular cycles passing though a type import should be cut at runtime as only used by TS. However I'm not 100% sure this is true for jest module loader, as I remember that in core, we had to move some imports around at some point to have code that was working in production behave correctly with the jest environment, so that doesn't help either. I can't find a proper way to progress further from here, and am currently stuck. |
|
So, as I couldn't think of anything better, I just decided to manually follow the leak from the
This package is importing the apm agent, which, if I'm not mistaken, patches native module at import time. I'm not sure why the leak was appearing only when importing both files in the test, but... To make sure, I reverted all my local changes in the // x-pack/plugins/actions/server/lib/action_executor.ts
-- import { withSpan } from '@kbn/apm-utils';No more leak for import { getAuthorizationModeBySource } from '../../actions/server/authorization/get_authorization_mode_by_source';
import { ensureSufficientLicense } from '../../actions/server/lib/ensure_sufficient_license';
describe('memory leak are hard', () => {
test('because we can', () => {
Object.keys(getAuthorizationModeBySource);
Object.keys(ensureSufficientLicense);
});
});No more leak for import { plugin } from '../../actions/server';
describe('memory leak are hard', () => {
test('because we can', () => {
Object.keys(plugin);
});
});Now... then I proceeded to remove all the Not sure to get it to be honest, as I previously tested running a test importing |
|
Removed all the stubs from That's it. No more leak. Now, we just have to decide how we handle this. Could we potentially add this |
In #117188 we're working to run Jest tests by their associated config file, parallelizing the work ourselves instead of relying on Jest workers (what FB does internally). This change is a blocker to per-PR code coverage reporting. We see instability with the workers and it hides some underlying issues. You can see OOM's in the current Jest tests for any CI run, as the Jest worker is automatically restarted and picked up where it left off. We shouldn't OOM, and we shouldn't have memory leaks as it could potentially be an issue in the application code.
The issue we're facing is with the
x-pack/plugins/security_solution/jest.config.jsconfig, as it seems to have a memory leak.There is a pretty good writeup here: https://chanind.github.io/javascript/2019/10/12/jest-tests-memory-leak.html
node --max-old-space-size=10000 --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage --config x-pack/plugins/security_solution/jest.config.jsMemory leaks
The text was updated successfully, but these errors were encountered: