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-error-serdes times out #52630

Open
aduh95 opened this issue Apr 21, 2024 · 9 comments
Open

test-error-serdes times out #52630

aduh95 opened this issue Apr 21, 2024 · 9 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI.

Comments

@aduh95
Copy link
Contributor

aduh95 commented Apr 21, 2024

@aduh95 aduh95 added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Apr 21, 2024
@github-actions github-actions bot added the linux Issues and PRs related to the Linux platform. label Apr 21, 2024
@targos
Copy link
Member

targos commented Apr 22, 2024

It's possible V8 12.3 introduced it.

@targos
Copy link
Member

targos commented Apr 22, 2024

I think I'm able to reproduce on my mac, but I got one timeout out of 1000 runs, which makes it difficult to debug.

@avivkeller
Copy link
Member

avivkeller commented Apr 22, 2024

I've gotten the timeout on several CI runs for #52509, so it's not system specific.

@avivkeller avivkeller removed the linux Issues and PRs related to the Linux platform. label Apr 22, 2024
@mhdawson
Copy link
Member

@joyeecheung
Copy link
Member

Report from nodejs/reliability#842

Reason parallel/test-error-serdes
Type JS_TEST_FAILURE
Failed PR 26 (https://github.com/nodejs/node/pull/52584/, https://github.com/nodejs/node/pull/52365/, https://github.com/nodejs/node/pull/52545/, https://github.com/nodejs/node/pull/52573/, https://github.com/nodejs/node/pull/49709/, https://github.com/nodejs/node/pull/52592/, https://github.com/nodejs/node/pull/52505/, https://github.com/nodejs/node/pull/51050/, https://github.com/nodejs/node/pull/52611/, https://github.com/nodejs/node/pull/52435/, https://github.com/nodejs/node/pull/51340/, https://github.com/nodejs/node/pull/51657/, https://github.com/nodejs/node/pull/52616/, https://github.com/nodejs/node/pull/52625/, https://github.com/nodejs/node/pull/52347/, https://github.com/nodejs/node/pull/52465/, https://github.com/nodejs/node/pull/52595/, https://github.com/nodejs/node/pull/52108/, https://github.com/nodejs/node/pull/52372/, https://github.com/nodejs/node/pull/52624/, https://github.com/nodejs/node/pull/52164/, https://github.com/nodejs/node/pull/52577/, https://github.com/nodejs/node/pull/52645/, https://github.com/nodejs/node/pull/52047/, https://github.com/nodejs/node/pull/52509/, https://github.com/nodejs/node/pull/52632/)
Appeared test-digitalocean-ubuntu2204_sharedlibs_container-x64-6, test-ibm-ubuntu2204_sharedlibs_container-x64-3, test-digitalocean-ubuntu2204_sharedlibs_container-x64-9, test-digitalocean-ubuntu2204_sharedlibs_container-x64-8, test-digitalocean-ubuntu2204_sharedlibs_container-x64-10, test-digitalocean-ubuntu2204_sharedlibs_container-x64-1, test-ibm-ubuntu2204_sharedlibs_container-x64-5, test-ibm-ubuntu2204_sharedlibs_container-x64-1, test-osuosl-ubuntu2004_container-armv7l-1, test-orka-macos11-x64-2, test-digitalocean-alpine319_container-x64-2, test-azure_msft-win11_vs2022-x64-4, test-digitalocean-alpine319_container-x64-1, test-ibm-ubi81_container-x64-1, test-equinix-debian11_container-armv7l-2, test-equinix-ubuntu2004_container-armv7l-1, test-digitalocean-ubuntu2204_sharedlibs_container-x64-4, test-digitalocean-rhel9-x64-1, test-digitalocean-alpine318_container-x64-1, test-digitalocean-ubuntu2204_sharedlibs_container-x64-3, test-ibm-rhel8-x64-1, test-digitalocean-ubuntu2204_sharedlibs_container-x64-5, test-ibm-ubuntu2204_sharedlibs_container-x64-2, test-ibm-ubuntu2204_sharedlibs_container-x64-4, test-digitalocean-ubuntu2204_sharedlibs_container-x64-7, test-equinix-rhel8_container-arm64-1, test-equinix-ubuntu2004_container-arm64-2, test-orka-macos11-x64-1, test-osuosl-ubuntu2004_sharedlibs_container-arm64-1
First CI https://ci.nodejs.org/job/node-test-pull-request/58526/
Last CI https://ci.nodejs.org/job/node-test-pull-request/58617/
Example
not ok 1397 parallel/test-error-serdes
  ---
  duration_ms: 120079.51800
  severity: fail
  exitcode: -15
  stack: |-
    timeout
  ...

@joyeecheung
Copy link
Member

joyeecheung commented Apr 23, 2024

On container builds where this does pass, the test finishes in 3-5 seconds, so this is likely a bug with V8.

@joyeecheung
Copy link
Member

Looking at other recent flakes that are at the top of the list, it seems the V8 update introduced quite a lot of timeouts, it could be some kind of deadlock.

@joyeecheung
Copy link
Member

Another clue - I don't see this flaking in the V8 CI (Linux x64 only), so it should be our bug.

@LiviaMedeiros
Copy link
Contributor

Not sure what's the root cause, but it could be that the update made exceeding the max stack size less stable.

let e = cycle(new ErrorWithCyclicCause('Error with cause'));
while (e.cause) {
e = e.cause;
depth++;
}
assert(depth > 1);

When I run this part with Node.js v20.12.1, I observe pretty much reproducible depth === 1542.
When I run it with current main build, the cycle ends with fluctuating numbers like 1717, 1649, etc.
Increasing stack size leads to exponential increase in time, e.g. node --stack-size=4096 test/parallel/test-error-serdes.js easily exceeds 120 seconds for me (ulimit -s is 8192).
Maybe under some "lucky" conditions this cycle can go abnormally deep?

aduh95 added a commit that referenced this issue Apr 28, 2024
PR-URL: #52739
Refs: #52630
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
aduh95 added a commit that referenced this issue Apr 29, 2024
PR-URL: #52739
Refs: #52630
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
marco-ippolito pushed a commit that referenced this issue May 2, 2024
PR-URL: #52739
Refs: #52630
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
marco-ippolito pushed a commit that referenced this issue May 3, 2024
PR-URL: #52739
Refs: #52630
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
sophoniie pushed a commit to sophoniie/node that referenced this issue Jun 20, 2024
PR-URL: nodejs#52739
Refs: nodejs#52630
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
bmeck pushed a commit to bmeck/node that referenced this issue Jun 22, 2024
PR-URL: nodejs#52739
Refs: nodejs#52630
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
jasnell added a commit that referenced this issue Sep 11, 2024
Hopefully reduces the run time and the likelihood of the test
failing with a flaky timeout error.

remove test-error-serdes from flaky list

move test-error-serdes to sequential

PR-URL: #54840
Refs: #52630
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
aduh95 pushed a commit that referenced this issue Sep 12, 2024
Hopefully reduces the run time and the likelihood of the test
failing with a flaky timeout error.

remove test-error-serdes from flaky list

move test-error-serdes to sequential

PR-URL: #54840
Refs: #52630
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
aduh95 pushed a commit that referenced this issue Sep 13, 2024
Hopefully reduces the run time and the likelihood of the test
failing with a flaky timeout error.

remove test-error-serdes from flaky list

move test-error-serdes to sequential

PR-URL: #54840
Refs: #52630
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
aduh95 pushed a commit that referenced this issue Sep 13, 2024
Hopefully reduces the run time and the likelihood of the test
failing with a flaky timeout error.

remove test-error-serdes from flaky list

move test-error-serdes to sequential

PR-URL: #54840
Refs: #52630
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
Hopefully reduces the run time and the likelihood of the test
failing with a flaky timeout error.

remove test-error-serdes from flaky list

move test-error-serdes to sequential

PR-URL: nodejs#54840
Refs: nodejs#52630
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
Hopefully reduces the run time and the likelihood of the test
failing with a flaky timeout error.

remove test-error-serdes from flaky list

move test-error-serdes to sequential

PR-URL: nodejs#54840
Refs: nodejs#52630
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants