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.

Sign up for GitHub

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

Jump to bottom

test: investigate flakiness of test-net-write-after-close #13597

Closed
refack opened this issue Jun 10, 2017 · 5 comments
Closed

test: investigate flakiness of test-net-write-after-close #13597

refack opened this issue Jun 10, 2017 · 5 comments
Labels
net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests.

Comments

@refack
Copy link
Contributor

refack commented Jun 10, 2017 •
edited by gibfahn
Loading

  • Version: master
  • Platform: freeBSD
  • Subsystem: test,net
985	parallel/test-net-write-after-close	
duration_ms	60.143
severity	fail
stack	timeout

https://ci.nodejs.org/job/node-test-commit-freebsd/9623/nodes=freebsd10-64/tapResults/
@refack refack added net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests. labels Jun 10, 2017
@refack refack changed the title test: investigate falkiness of test: investigate flakiness of test-net-write-after-close Jun 10, 2017
@refack refack mentioned this issue Jun 11, 2017
Merged
4 tasks
@MylesBorins
Copy link
Contributor

MylesBorins commented Jul 18, 2017 •
edited by gibfahn
Loading

I'm seeing this rear its head in v6.x now

not ok 828 parallel/test-net-write-after-close
  ---
  duration_ms: 60.80
  severity: fail
  stack: |-
    timeout
  ...

Should we add to known flakes?

@refack
Copy link
Contributor Author

refack commented Jul 18, 2017

/cc @nodejs/platform-freebsd @nodejs/streams

@Trott
Copy link
Member

Trott commented Jul 19, 2017

Looks like it's the usual thing with FreeBSD flakes in CI, which is a race condition triggered by load.

If you shorten the hardcoded 250ms timeout, you can trigger the race faster/easier.

Source of the problem seems to be that the connection callback can fire after 250ms on a loaded machine.

PR coming shortly to fix this...

@Trott Trott mentioned this issue Jul 19, 2017
Closed
2 tasks
@Trott
Copy link
Member

Trott commented Jul 19, 2017

Proposed fix in #14361

@refack
Copy link
Contributor Author

refack commented Jul 19, 2017

Looks like it's the usual thing with FreeBSD flakes in CI, which is a race condition triggered by load.

If you shorten the hardcoded 250ms timeout, you can trigger the race faster/easier.

Source of the problem seems to be that the connection callback can fire after 250ms on a loaded machine.

PR coming shortly to fix this...

Should we start a "raw numbers to common.platformTimeout" tracking issue?

Trott added a commit to Trott/io.js that referenced this issue Jul 19, 2017
@Trott
test: fix flaky test-net-write-after-close
64794dd 
Replace 250ms timer with event-based logic to make test robust.

Fixes: nodejs#13597
@Trott Trott closed this as completed in 43bd47c Jul 21, 2017
addaleax pushed a commit that referenced this issue Jul 22, 2017
@Trott @addaleax
test: fix flaky test-net-write-after-close
200c65a 
Replace 250ms timer with event-based logic to make test robust.

PR-URL: #14361
Fixes: #13597
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this issue Jul 24, 2017
@Trott @Fishrock123
test: fix flaky test-net-write-after-close
9b22acc 
Replace 250ms timer with event-based logic to make test robust.

PR-URL: #14361
Fixes: #13597
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@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
Assignees
No one assigned
Labels
net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: fix flaky test-net-write-after-close Trott/io.js
3 participants
@refack @MylesBorins @Trott