nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
104.97k stars 28.42k forks source link

Graceful close for HTTP2 sessions #34265

Open szmarczak opened 3 years ago

szmarczak commented 3 years ago

Is your feature request related to a problem? Please describe.

IMO it would be very nice to have session.gracefullyClose() so pending streams can be finished but no more streams can be created on this session ~(or closing is canceled when session.request() is called)~. I'm developing some sort of a pool (agent?) for http2, and I need to gracefully close a session when it becomes a subset of another session.

I do NOT want to notify the server because they can reject current streams with REFUSED_STREAM which means it hasn't processed the request yet and it is safe to retry. But the request body could have been uploaded in that case.

Describe the solution you'd like

See the paragraph above.

Describe alternatives you've considered I can implement this on my own, eg. gracefullyClose(session).

targos commented 3 years ago

@nodejs/http2

mcollina commented 3 years ago

I agree, this would be a good feature indeed. There does not seem to be much interest in http2 lately... it really need somebody to be willing to implement this.

cc @jasnell

pmj642 commented 3 years ago

Hey, I wanted to take this issue. So, I read about http2, http2server, http2session in node. And, I found the following in API docs. https://nodejs.org/api/http2.html#http2_http2session_close_callback

So, the http2session.close() method does the exact same thing that's being asked. Am I missing something?

@mcollina

mcollina commented 3 years ago

@szmarczak will session.close() be enough?

szmarczak commented 3 years ago

@mcollina It sends a GOAWAY frame. The server may reject the stream with REFUSED_STREAM - even though the body has been already uploaded. I think it would be much better to manually wait until the streams complete then send GOAWAY.

szmarczak commented 3 years ago

clients SHOULD NOT emit new requests on any connection whose Origin Set is a proper subset of another connection's Origin Set, and they SHOULD close it once all outstanding requests are satisfied.

Source: https://tools.ietf.org/html/rfc8336#section-2.4

szmarczak commented 3 years ago

So, the http2session.close() method does the exact same thing that's being asked.

@pmj642 No, I asked not to send a GOAWAY frame (well we can but once all streams complete). http2session.close() does :)

devsnek commented 3 years ago

Maybe something like session1.coalsece(session2) could represent this better, while also cleaning up state to the authoritative session?

szmarczak commented 3 years ago

Yep, that would definitely do!

pmj642 commented 3 years ago

@szmarczak @devsnek I read the relevant RFC sections and also went through the code for http2session class. I'm actually trying to grasp the context.

So, if a http2 client wants to close its connection due to its Origin Set being a subset of another connection from the client side. But the server had initiated a new stream S1 and let's say that a few frames from that stream had been processed by the client before it decided to close the connection and send the GOAWAY frame. So, as soon as the client sends a GOAWAY frame with the id of the last processed stream S0, it wont process any more frames of the S1 stream and will return REFUSED_STREAM. The server will have to send this data in a new connection. This causes some lost frames from S1 stream.

So, before sending the GOAWAY frame the client could have changed the state of session to closed which would have allowed the processing of the existing streams while refusing any new streams. And once all streams had processed, it could then send the GOAWAY frame to the server.

Is this the correct interpretation?

Trott commented 3 years ago

@nodejs/http2 Is anyone able to confirm or correct @pmj642's understanding?

mcollina commented 3 years ago

Ping @jasnell

mcollina commented 3 years ago

From what I know and a brief skim, @pmj642 is correct

ktfth commented 3 years ago

I can help on this issue?

mcollina commented 3 years ago

I can help on this issue?

Sure, send a PR and tag us!

ktfth commented 3 years ago

How can i get started, have a draft for that?

mcollina commented 3 years ago

Follow https://www.nodetodo.org/ to get started. It's probably not so easy to start with this - it might involve some C++ development and spec reading.

ktfth commented 3 years ago

@mcollina Thank you

tiptronic commented 1 month ago

Just a question: was there any progress on this?