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

[v8.x backport] http2: simplify timeout tracking #23774

Closed
wants to merge 1 commit into from
Closed

[v8.x backport] http2: simplify timeout tracking #23774

wants to merge 1 commit into from

Conversation

kjin
Copy link
Contributor

@kjin kjin commented Oct 20, 2018 •
edited

See #19206

cc/ @BethGriggs @addaleax

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@addaleax @kjin
http2: simplify timeout tracking
c6b2a7b 
There’s no need to reset the chunk counter for every write.

PR-URL: nodejs#19206
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. v8.x labels Oct 20, 2018
@BethGriggs
Copy link
Member

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

@MylesBorins
Copy link
Member

MylesBorins commented Oct 31, 2018 •
edited

one more ci: https://ci.nodejs.org/job/node-test-pull-request/18265/

@MylesBorins
Copy link
Member

Getting failures

13:58:13   ...
13:58:13 not ok 2150 sequential/test-timers-throw-reschedule # TODO : Fix flaky test
13:58:13   ---
13:58:13   duration_ms: 0.211
13:58:13   severity: flaky
13:58:13   exitcode: 1
13:58:13   stack: |-
13:58:13     assert.js:43
13:58:13       throw new errors.AssertionError(obj);
13:58:13       ^
13:58:13     
13:58:13     AssertionError [ERR_ASSERTION]: false == true
13:58:13         at Timeout.common.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/sequential/test-timers-throw-reschedule.js:20:5)
13:58:13         at Timeout._onTimeout (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:479:15)
13:58:13         at ontimeout (timers.js:498:11)
13:58:13         at Timer.unrefdHandle (timers.js:611:5)

MylesBorins pushed a commit that referenced this pull request Oct 31, 2018
@addaleax @MylesBorins
http2: simplify timeout tracking
7f34c27 
There’s no need to reset the chunk counter for every write.

Backport-PR-URL: #23774
PR-URL: #19206
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 31, 2018
Closed
@MylesBorins
Copy link
Member

hmmm, that test is supposed to be marked flaky... strange going to go ahead and land this

landed in 7f34c27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kjin @BethGriggs @MylesBorins @addaleax @nodejs-github-bot