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-http2-session-timeout flaky on FreeBSD #15326

Closed
MylesBorins opened this issue Sep 11, 2017 · 11 comments
Closed

test-http2-session-timeout flaky on FreeBSD #15326

MylesBorins opened this issue Sep 11, 2017 · 11 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. http2 Issues or PRs related to the http2 subsystem. test Issues and PRs related to the tests.

Comments

@MylesBorins
Copy link
Contributor

This has been showing up a couple times in the v8.5.0-proposal testing

not ok 854 parallel/test-http2-session-timeout
  ---
  duration_ms: 0.957
  severity: fail
  stack: |-
    (node:89251) ExperimentalWarning: The http2 module is an experimental API.
    assert.js:41
      throw new errors.AssertionError({
      ^
    
    AssertionError [ERR_ASSERTION]: function should not have been called
        at Http2Server.mustNotCall (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/common/index.js:560:12)
        at emitOne (events.js:115:13)
        at Http2Server.emit (events.js:210:7)
        at process.nextTick (internal/http2/core.js:2282:17)
        at _combinedTickCallback (internal/process/next_tick.js:131:7)
        at process._tickCallback (internal/process/next_tick.js:180:9)
  ...
ok 855 

/cc @nodejs/platform-freebsd

@MylesBorins
Copy link
Contributor Author

Digging into the test it would appear that this is a timout

server.on('timeout', common.mustNotCall());

@BridgeAR
Copy link
Member

Ref #15167 (comment)

@mscdex mscdex added freebsd Issues and PRs related to the FreeBSD platform. http2 Issues or PRs related to the http2 subsystem. test Issues and PRs related to the tests. labels Sep 11, 2017
@Trott
Copy link
Member

Trott commented Sep 11, 2017

I am able to reproduce this trivially with:

tools/test.py -j 96 --repeat 192 test/parallel/test-http2-session-timeout.js

(Note that at this time you need to apply the patch from #15300 or back out c8a389e to use the test runner like that.)

@Trott
Copy link
Member

Trott commented Sep 11, 2017

This is not FreeBSD-specific. The test is just straight-up not tolerant of concurrency. Removing the freebsd label.

@Trott Trott added flaky-test Issues and PRs related to the tests with unstable failures on the CI. and removed freebsd Issues and PRs related to the FreeBSD platform. labels Sep 11, 2017
@Trott
Copy link
Member

Trott commented Sep 11, 2017

Aside: This test uses the .timeout property of the Http2Server class but that property (and all properties on that class) is undocumented. @nodejs/http2 @nodejs/documentation

@Trott
Copy link
Member

Trott commented Sep 11, 2017

Simplest solution is to bump up serverTimeout. Changing it to 1200 (which is exactly 1/100 of the default value) allows me to run 96 simultaneous copies of the test locally reliably. Previously, I was getting errors sometimes with as few as 8 or 16 concurrent tests, and reliably with 32 concurrent tests.

PR coming momentarily.

@Trott
Copy link
Member

Trott commented Sep 11, 2017

Fix (or at least mitigation) is in #15328

@mhdawson
Copy link
Member

@apapirovski
Copy link
Member

apapirovski commented Sep 14, 2017

Sorry, everyone. We're trying to get this nailed down. If someone wants to start a stress test for the latest commit apapirovski@3103f89 that would be appreciated!

Edit: Sorry, this is the PR #15338

@Trott
Copy link
Member

Trott commented Sep 14, 2017

If someone wants to start a stress test

I'm on it.

Trott added a commit to Trott/io.js that referenced this issue Sep 14, 2017
Increase server timeout to reduce likelihood of triggering race
conditions.

Fixes: nodejs#15326
@Trott
Copy link
Member

Trott commented Sep 16, 2017

Still failing on FreeBSD with some frequency.Example: https://ci.nodejs.org/job/node-test-commit-freebsd/11602/nodes=freebsd10-64/console

I'll open a PR to move it to sequential as-is (to get it working right away) and it can be moved back in #15338 whenever that lands.

Trott added a commit to Trott/io.js that referenced this issue Sep 16, 2017
Move concurrency-sensitive test to `sequential` directory. It is failing
somewhat frequently on FreeBSD in CI.

Fixes: nodejs#15326
apapirovski added a commit to apapirovski/node that referenced this issue Sep 16, 2017
Increase server timeout, reduce frequency of calls and
unbind timeout after runs are done in order to avoid
race conditions. Temporarily moved to sequential.

Fixes: nodejs#15326
jasnell pushed a commit that referenced this issue Sep 20, 2017
Increase server timeout, reduce frequency of calls and
unbind timeout after runs are done in order to avoid
race conditions. Temporarily moved to sequential.

Fixes: #15326
PR-URL: #15338
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Qard pushed a commit to Qard/ayo that referenced this issue Sep 21, 2017
Increase server timeout, reduce frequency of calls and
unbind timeout after runs are done in order to avoid
race conditions. Temporarily moved to sequential.

Fixes: nodejs/node#15326
PR-URL: nodejs/node#15338
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Qard pushed a commit to Qard/ayo that referenced this issue Sep 21, 2017
Increase server timeout, reduce frequency of calls and
unbind timeout after runs are done in order to avoid
race conditions. Temporarily moved to sequential.

Fixes: nodejs/node#15326
PR-URL: nodejs/node#15338
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
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. http2 Issues or PRs related to the http2 subsystem. test Issues and PRs related to the tests.
Projects
None yet
6 participants