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(ci): allocate more memory to fix test_vscode MONGOSH-1892 #2239

Merged
merged 15 commits into from
Nov 22, 2024

Conversation

gagik
Copy link
Contributor

@gagik gagik commented Oct 25, 2024

Fixes test_vscode which had started to run out of memory by bumping the max allocation size.

@gagik gagik marked this pull request as draft October 25, 2024 13:03
@gagik gagik changed the title WIP: fix test_vscode fix(ci): allocate more memory to fix test_vscode VSCODE-650 Nov 22, 2024
@gagik gagik changed the title fix(ci): allocate more memory to fix test_vscode VSCODE-650 fix(ci): allocate more memory to fix test_vscode MONGOSH-1892 Nov 22, 2024
@gagik gagik marked this pull request as ready for review November 22, 2024 12:26
Copy link
Contributor

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just in case you happen to know, have you figured out what part of the test takes this much memory? I think this is perfectly fine as a workaround but it's slighly concerning that the extension tests take this much time, and I feel like this might point in the direction of a memory leak that's bad enough to cause real-world trouble

@gagik
Copy link
Contributor Author

gagik commented Nov 22, 2024

@addaleax Hm yeah, I just tried to create and use a mocha reporter which kept track of process.memoryUsage().heapUsed before and after a test and did not see anything >10mb. Probably kind of a flawed method but I am wondering if the memory usage issue is just coming from the electron being spun up? I created a VSCODE ticket to track this.

@gagik gagik merged commit 8bf4c46 into main Nov 22, 2024
80 of 86 checks passed
@gagik gagik deleted the gagik/fix-vscode-test branch November 22, 2024 13:03
@lerouxb
Copy link
Contributor

lerouxb commented Nov 22, 2024

My hunch is that it happens when something fails. In my experience you often get something trying to pretty-print gigabytes of something on error. Or similar :P Could even be circular references not handled.

nirinchev added a commit that referenced this pull request Dec 2, 2024
* main: (34 commits)
  chore: update auto-generated files [skip actions]
  chore: update auto-generated files [skip actions]
  chore(ci): use github app tokens instead of bot user (#2282)
  chore: update auto-generated files [skip actions]
  chore(ci): cleanup auto generated file workflow MONGOSH-1927 (#2281)
  chore: update auto-generated files
  chore(ci): use a github app for the generated files credentials MONGOSH-1927 (#2280)
  chore(deps): bump driver, bson, oidc-plugin to latest MONGOSH-1916 (#2279)
  chore(ci): login to docker (#2277)
  chore(ci): don't download chrome for cron tasks (#2278)
  chore: update node.js (#2270)
  chore: push auto-generated files directly instead of using PRs (#2275)
  fix(ci): allocate more memory to fix test_vscode MONGOSH-1892 (#2239)
  chore: update auto-generated files (#2276)
  feat(shell-api): add options in stream processor start, stop, and drop MONGOSH-1920 (#2274)
  chore: update auto-generated files (#2273)
  chore: update auto-generated files (#2272)
  fix: include nonce in oidc request by default MONGOSH-1905 MONGOSH-1917 (#2269)
  chore(ci): filter for explictily set variants in unit tests (#2271)
  chore: update auto-generated files (#2268)
  ...
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