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

Why http2session.socket.bufferSize is always zero? #21631

Closed
micooz opened this issue Jul 3, 2018 · 4 comments
Closed

Why http2session.socket.bufferSize is always zero? #21631

micooz opened this issue Jul 3, 2018 · 4 comments
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@micooz
Copy link

micooz commented Jul 3, 2018

  • Version: v10.4.1
  • Platform: macOS 10.13.5
  • Subsystem: net,http2

I encountered a problem that I couldn't get expected bufferSize from http2session, http2session.socket.bufferSize is always zero.

Reproduce Code

I write a test similar to test-net-buffersize.js:

const assert = require('assert');
const http2 = require('http2');

const server = http2.createServer();

server.listen(0, () => {
  const session = http2.connect(`http://localhost:${server.address().port}`);
  const stream = session.request({ ':path': '/' }, { endStream: false });

  for (let i = 1; i < 10; i++) {
    assert(stream.writable);
    stream.write('a');
    assert.strictEqual(session.socket.bufferSize, i); // AssertionError
  }

  stream.close();
});

Is it correct way to obtain bufferSize from http2 session? I need this value to throttle uploads like net.Socket do.

@ChALkeR ChALkeR added the http2 Issues or PRs related to the http2 subsystem. label Jul 3, 2018
@jasnell
Copy link
Member

jasnell commented Aug 10, 2018

This is due to the special way that http2 sessions interact with the socket at the native (c++) layer rather than the javascript layer. The socket object's js interface is not used directly, and therefore the bufferSize is not set. We can investigate a way of enabling that.

@jasnell
Copy link
Member

jasnell commented Sep 21, 2018

@addaleax ... do you have any thoughts on how we could make bufferSize work on a socket bound to an http2session?

@oyyd
Copy link
Contributor

oyyd commented Oct 16, 2018

According to the net module, bufferSize is:

node/lib/net.js

Line 518 in deaddd2

return this[kLastWriteQueueSize] + this.writableLength;

As Http2Session and Http2Stream have internally maintained writeQueueSize which is actually kLastWriteQueueSize, I think, maybe we can make bufferSize work on http2 module itself.

oyyd added a commit to oyyd/node that referenced this issue Oct 17, 2018
This commit adds `bufferSize` for `Http2Stream`.

Refs: nodejs#21631
Trott pushed a commit to Trott/io.js that referenced this issue Nov 6, 2018
This commit adds `bufferSize` for `Http2Stream`.

Refs: nodejs#21631

PR-URL: nodejs#23711
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Nov 6, 2018
This commit adds `bufferSize` for `Http2Stream`.

Refs: #21631

PR-URL: #23711
Reviewed-By: James M Snell <[email protected]>
@apapirovski
Copy link
Member

The PR for this has landed so I believe we can close this.

BethGriggs pushed a commit that referenced this issue Apr 8, 2019
This commit adds `bufferSize` for `Http2Stream`.

Refs: #21631

PR-URL: #23711
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants