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

HTTP/2 requests eventually start throwing NGHTTP2_ENHANCE_YOUR_CALM errors #27416

Closed
akukas opened this issue Apr 25, 2019 · 5 comments
Closed

HTTP/2 requests eventually start throwing NGHTTP2_ENHANCE_YOUR_CALM errors #27416

akukas opened this issue Apr 25, 2019 · 5 comments
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@akukas
Copy link
Contributor

akukas commented Apr 25, 2019

  • Version: v12.0.0, v11.14.0
  • Platform: Windows 10 64bit, Ubuntu 18.04.2
  • Subsystem: http2

I've experienced an issue similar to #23116 on more recent Node versions. After a certain amount of requests are processed in the same http2 session, further requests will start throwing errors:

Error [ERR_HTTP2_STREAM_ERROR]: Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM
    at ClientHttp2Stream._destroy (internal/http2/core.js:1951:13)
    at ClientHttp2Stream.destroy (internal/streams/destroy.js:37:8)
    at ClientHttp2Stream.[kMaybeDestroy] (internal/http2/core.js:1967:12)
    at Http2Stream.onStreamClose (internal/http2/core.js:388:26)

I've used https://gist.github.com/akukas/46f5a850bb53cd95a887df16b75fd8a4 for testing. With maxSessionMemory set to 1 (the minimum value), the client session will make it through ~48k requests before failing. Increasing the memory limit will increase the number of requests proportionally. In Node v10.15.3, the test script will work as expected, running indefinitely.

Something else I noticed while watching the process in Task Manager/htop is that memory usage of the process remains static. Could it be that the http2 session isn't actually running out of memory, but there's just a memory usage tracking issue making it think it is?
@bnoordhuis bnoordhuis added the http2 Issues or PRs related to the http2 subsystem. label Apr 26, 2019
@crystalin
Copy link

I've been running into the same issue. Not sure how to handle it for now.

@mrfelton
Copy link

Also seeing this issue with latest node v12. Increasing maxSessionMemory only prolongs the time until the issue occurs.

@akukas
Copy link
Contributor Author

akukas commented May 21, 2019

Tried this out across different versions, the issue is present on Node v11.11.0 and up.

Taking a quick look at the changelog, 8a551b9d3b appears to be a possible cause, but I don't know what it could be.
@addaleax could you take a look at this?

@addaleax
Copy link
Member

@akukas Looks like that isn’t the issue, but instead my #26207 is :) I’ll take a look.

addaleax added a commit to addaleax/node that referenced this issue May 26, 2019
@addaleax
http2: fix tracking received data for maxSessionMemory
4b5176f 
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#27416
Refs: nodejs#26207
@addaleax addaleax mentioned this issue May 26, 2019
Closed
3 tasks
@addaleax
Copy link
Member

Yeah, this is 100 % on me, sorry. @akukas Thank you for reporting this and pinging me!

#27914 should address this.

targos pushed a commit that referenced this issue May 31, 2019
@addaleax @targos
http2: fix tracking received data for maxSessionMemory
83aaef8 
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>
@mrfelton mrfelton mentioned this issue Jun 15, 2019
Closed
3 tasks
MarshallOfSound pushed a commit to electron/node that referenced this issue Jun 18, 2019
@addaleax @MarshallOfSound
http2: fix tracking received data for maxSessionMemory
0a300f6 
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 pushed a commit to electron/node that referenced this issue Jul 12, 2019
@addaleax @deepak1556
http2: fix tracking received data for maxSessionMemory
1ca642c 
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>
@0xmaayan 0xmaayan mentioned this issue Mar 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

http2: fix tracking received data for maxSessionMemory addaleax/node
5 participants
@mrfelton @bnoordhuis @crystalin @addaleax @akukas