-
-
Notifications
You must be signed in to change notification settings - Fork 250
Fix MongoDB container excessive CPU usage #1146
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
Conversation
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This reduces CPU usage for me, woo hoo! Sample output of docker stats (in a test described here), after this PR: Compare with before this PR: |
|
@digital88 any thoughts on what sort of regression test could've caught this? I have no design ideas here, but I figure the question could be productive to ask. (And thanks so much for your light-speed fix for this issue!) |
Good to hear it's mitigated! Probably something like your test example, but this problem is also tied to mongo TC implementation. Because TC supported mongo with mandatory replica set, I had to keep this implementation working when I added user+password auth. Didn't catch that low retry interval would make it constantly re-create rs and consume CPU. As side note I still think replica set initialization should be separate method on started mongo container, not something that is forced by default when starting container. |
|
Hi @digital88, thanks for the fix! Are there any other changes you want to make or we're good to merge? A test would've been nice but I also don't see how we can test that container CPU utilisation is "reasonable", consistently across multiple machines |
Yes, I think we are good to merge, @CodingCanuck confirmed it's fixed. I'm too not sure how even to begin approach this regression with tests. We somehow need to benchmark external service (docker or podman) while running our code. |
I completely agree: I can't imagine a pass/fail "unit"-style test that would be productive here.
I can't imagine regression / correctness tests, but I can imagine performance tests that generally tracks cost / compute changes across PRs. Changes in cost from any given PR can then be optionally investigated, but higher CPU doesn't necessarily mean worse (since most new features will consume some CPU). I'd imagine an auto-generated report that could be attached to each PR, saying "this increased CPU by X% in benchmark case Y", and the PR reviewer could decide whether that's desired / expected. I'm not familiar with the testcontainers repo and I don't know what priorities it has, but note that the XZ Utils backdoor that made news cycles last year was caught by having a performance testing suite: https://en.wikipedia.org/wiki/XZ_Utils_backdoor#Background
So I'd consider this as a completely different issue that's way out of scope for this particular regression, but the only recommendation I could think of making here is cross-product performance/CPU tests, where you can optionally investigate spikes in CPU consumption. (And without knowing testcontainers well, I'd guess that it's likely not worth the effort if you don't see this kind of regression often) |
This fixes #1145
@CodingCanuck can you please try this? My cpu usage became reasonable. I upped HC retry to 5s and made sure we first check replica set status with
rs.status()(mongo docs says it throws if no rs was initiated) beforers.initiate(). For mongo v4 and below I kept previous healthcheck command.Also, had to touch
.prettierrcandtest-helper.tsbecause I'm on Windows and\njust breaks everything here :)