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

test: fix flaky test-memory-usage #31602

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Feb 1, 2020

abe6a2e introduced a test that verifies that ArrayBuffer
allocations are tracked. However, V8 supports concurrent freeing
of such allocations on background threads, meaning that the results
may be subject to race conditions sometimes.

Disabling concurrent freeing makes the test pass consistently.

Refs: #31550

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

abe6a2e introduced a test that verifies that ArrayBuffer
allocations are tracked. However, V8 supports concurrent freeing
of such allocations on background threads, meaning that the results
may be subject to race conditions sometimes.

Disabling concurrent freeing makes the test pass consistently.

Refs: nodejs#31550
@addaleax addaleax added test Issues and PRs related to the tests. process Issues and PRs related to the process subsystem. flaky-test Issues and PRs related to the tests with unstable failures on the CI. fast-track PRs that do not need to wait for 48 hours to land. labels Feb 1, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 1, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/28851/

👍 this comment to approve fast-tracking.

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 1, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@addaleax
Copy link
Member Author

addaleax commented Feb 1, 2020

After this lands I’ll go through the CI runs and open a PR to mark the failed Windows test as flaky, there’s half a dozen red ones just in the runs for this PR 😨

@addaleax
Copy link
Member Author

addaleax commented Feb 1, 2020

Landed in 06deb94

@addaleax addaleax closed this Feb 1, 2020
addaleax added a commit that referenced this pull request Feb 1, 2020
abe6a2e introduced a test that verifies that ArrayBuffer
allocations are tracked. However, V8 supports concurrent freeing
of such allocations on background threads, meaning that the results
may be subject to race conditions sometimes.

Disabling concurrent freeing makes the test pass consistently.

Refs: #31550

PR-URL: #31602
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax addaleax deleted the fix-test-memory-usage branch February 1, 2020 20:15
addaleax added a commit to addaleax/node that referenced this pull request Feb 1, 2020
Basically, any of the tests that failed in the runs for
nodejs#31602 which was not already
marked as flaky.
addaleax added a commit that referenced this pull request Feb 1, 2020
Basically, any of the tests that failed in the runs for
#31602 which was not already
marked as flaky.

PR-URL: #31606
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
codebytere pushed a commit that referenced this pull request Feb 17, 2020
abe6a2e introduced a test that verifies that ArrayBuffer
allocations are tracked. However, V8 supports concurrent freeing
of such allocations on background threads, meaning that the results
may be subject to race conditions sometimes.

Disabling concurrent freeing makes the test pass consistently.

Refs: #31550

PR-URL: #31602
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Feb 17, 2020
Basically, any of the tests that failed in the runs for
#31602 which was not already
marked as flaky.

PR-URL: #31606
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@codebytere codebytere mentioned this pull request Feb 17, 2020
codebytere pushed a commit that referenced this pull request Mar 15, 2020
Basically, any of the tests that failed in the runs for
#31602 which was not already
marked as flaky.

PR-URL: #31606
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
Basically, any of the tests that failed in the runs for
#31602 which was not already
marked as flaky.

PR-URL: #31606
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 30, 2020
Basically, any of the tests that failed in the runs for
#31602 which was not already
marked as flaky.

PR-URL: #31606
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@targos
Copy link
Member

targos commented Apr 18, 2020

Depends on #31550 to land on v12.x

@targos targos removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v12.x labels Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
abe6a2e introduced a test that verifies that ArrayBuffer
allocations are tracked. However, V8 supports concurrent freeing
of such allocations on background threads, meaning that the results
may be subject to race conditions sometimes.

Disabling concurrent freeing makes the test pass consistently.

Refs: nodejs#31550

PR-URL: nodejs#31602
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Apr 28, 2020
abe6a2e introduced a test that verifies that ArrayBuffer
allocations are tracked. However, V8 supports concurrent freeing
of such allocations on background threads, meaning that the results
may be subject to race conditions sometimes.

Disabling concurrent freeing makes the test pass consistently.

Refs: #31550

PR-URL: #31602
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. flaky-test Issues and PRs related to the tests with unstable failures on the CI. process Issues and PRs related to the process subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants