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

http2: fix tracking received data for maxSessionMemory #27914

Closed

Conversation

@addaleax
Copy link
Member

commented May 26, 2019

Track received data correctly. Specifically, for the buffer that
is used for receiving data, we previously would try to increment
the current memory usage by its length, and later decrement it
by that, but in the meantime the buffer had been turned over to V8
and its length reset to zero. This gave the impression that more and
more memory was consumed by the HTTP/2 session when it was in fact not.

Fixes: #27416
Refs: #26207

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Track received data correctly. Specifically, for the buffer that
is used for receiving data, we previously would try to increment
the current memory usage by its length, and later decrement it
by that, but in the meantime the buffer had been turned over to V8
and its length reset to zero. This gave the impression that more and
more memory was consumed by the HTTP/2 session when it was in fact not.

Fixes: #27416
Refs: #26207
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@Trott
Trott approved these changes May 27, 2019
@nodejs-github-bot

This comment has been minimized.

@addaleax

This comment has been minimized.

Copy link
Member Author

commented May 29, 2019

Landed in b84b4d8

@addaleax addaleax closed this May 29, 2019
@addaleax addaleax deleted the addaleax:http2-max-session-memory-tracking branch May 29, 2019
addaleax added a commit that referenced this pull request May 29, 2019
Track received data correctly. Specifically, for the buffer that
is used for receiving data, we previously would try to increment
the current memory usage by its length, and later decrement it
by that, but in the meantime the buffer had been turned over to V8
and its length reset to zero. This gave the impression that more and
more memory was consumed by the HTTP/2 session when it was in fact not.

Fixes: #27416
Refs: #26207

PR-URL: #27914
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos added a commit that referenced this pull request May 31, 2019
Track received data correctly. Specifically, for the buffer that
is used for receiving data, we previously would try to increment
the current memory usage by its length, and later decrement it
by that, but in the meantime the buffer had been turned over to V8
and its length reset to zero. This gave the impression that more and
more memory was consumed by the HTTP/2 session when it was in fact not.

Fixes: #27416
Refs: #26207

PR-URL: #27914
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos targos referenced this pull request Jun 3, 2019
MarshallOfSound added a commit to electron/node that referenced this pull request Jun 18, 2019
Track received data correctly. Specifically, for the buffer that
is used for receiving data, we previously would try to increment
the current memory usage by its length, and later decrement it
by that, but in the meantime the buffer had been turned over to V8
and its length reset to zero. This gave the impression that more and
more memory was consumed by the HTTP/2 session when it was in fact not.

Fixes: nodejs/node#27416
Refs: nodejs/node#26207

PR-URL: nodejs/node#27914
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
deepak1556 added a commit to electron/node that referenced this pull request Jul 12, 2019
Track received data correctly. Specifically, for the buffer that
is used for receiving data, we previously would try to increment
the current memory usage by its length, and later decrement it
by that, but in the meantime the buffer had been turned over to V8
and its length reset to zero. This gave the impression that more and
more memory was consumed by the HTTP/2 session when it was in fact not.

Fixes: nodejs/node#27416
Refs: nodejs/node#26207

PR-URL: nodejs/node#27914
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BethGriggs added a commit that referenced this pull request Aug 15, 2019
Refs: #27914

Backport-PR-URL: #29123
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs added a commit that referenced this pull request Aug 15, 2019
Refs: #27914

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@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
Projects
None yet
4 participants
You can’t perform that action at this time.