Skip to content

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

Closed
kjin wants to merge 1 commit intonodejs:v8.x-stagingfrom
kjin:8.x-19206
Closed

[v8.x backport] http2: simplify timeout tracking#23774
kjin wants to merge 1 commit intonodejs:v8.x-stagingfrom
kjin:8.x-19206

Conversation

@kjin
Copy link
Contributor

@kjin kjin commented Oct 20, 2018 •
edited
Loading

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

BethGriggs commented Oct 30, 2018

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

@MylesBorins
Copy link
Contributor

MylesBorins commented Oct 31, 2018 •
edited
Loading

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

@MylesBorins
Copy link
Contributor

MylesBorins commented Oct 31, 2018

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
Contributor

MylesBorins commented Oct 31, 2018

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.

5 participants

@kjin @BethGriggs @MylesBorins @addaleax @nodejs-github-bot