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

Draft: HTTPS Mux SessionState #991

Closed
wants to merge 31 commits into from
Closed

Conversation

Wonshtrum
Copy link
Member

This is the final stage of our proposition to support HTTP/2 in Sozu.
Taking full advantage of kawa translating Mux is a state that handles one frontend connection and up to N backend connections in a protocol-agnostic way. Protocol specifics are secluded in connection implementations which only sees its internal state, a shared Context, and a protocol-agnostic interface to its opposite endpoint.
The translation is smoothly and transparently handled by kawa while Mux treats connections all the same.

Note: I really think Mux is a good way to abstract a lot of cases and reduce the amount of code necessary to handle them. However, it might be overengineered and suboptimal for the most common cases (H1 -> H1 and H2 -> { H2, H2, ... }) since it always operates as if in the worst-case scenario ( H2 -> { H2, H1, ... } ).

@Keksoj
Copy link
Member

Keksoj commented Sep 12, 2023

That's awesome work, and an achievement.
We'll definitely have to update the kawa repository for build compatibity, before we dive into the review.

@Wonshtrum Wonshtrum force-pushed the mux_h1_h2_errors_and_retries branch from 489e590 to 60ee968 Compare September 14, 2023 18:45
Wonshtrum and others added 26 commits November 29, 2023 18:05
Signed-off-by: Eloi DEMOLIS <[email protected]>
- Rename Context
- Mutualize hpack decoder (one in Context, not per Connection)
- Separate streams in a new Streams struct
- Separate stream zero from the others
- PoC hpack decode from stream zero into another stream

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Implement Priority, PushPromise, GoAway and Continuation frame parsing
- Add kawa_h1::HttpContext to Streams
- Separate headers handling into pkawa module (will be relocated to
  Kawa)
- Use kawa_h1::editor callbacks to edit h2 headers and HttpContext

Signed-off-by: Eloi DEMOLIS <[email protected]>
Signed-off-by: Eloi DEMOLIS <[email protected]>
- Add routing capabilities to MuxSession
- Implement Data frame handling
- Fix handle_headers
- Don't store pseudo headers key in kawa
- Rename front and back Kawa fields to rbuffer, wbuffer

A MuxSession can now process an incoming H2 request, connect to the
corresponding backend, translate it in H1, send it and receive the H1
response. We still lack the capability to connect to H2 backends and
forward responses.

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Move H2 stream 0 out of Mux context into ConnectionH2 (so each H2
  connection can have its own unshared stream 0)
- Change pool errors propagation
- Add ack flag on Settings frame
- Add split methods on Stream (for borrowing reason)

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Move hpack Decoder back to individual ConnectionH2
- Add basic H2 kawa BlockConverter
- Add basic ConnectionH2 writable implementation

H2 headers are hard to work with. We should write our own implementation
of hpack to address the following issues:

- Use of transmute to change an hpack Decoder in Encoder
- No support for uppercased header names
- Duplication of header tables (one per connection)
- Aggressive decoding/reencoding

Signed-off-by: Eloi DEMOLIS <[email protected]>
- H1 Connection servers send connect event upon receiving the full
  request headers
- Add Readable interest to H1 and H2 clients, this avoids blocking on
  H1 close delimited requests
- Header names are lowercased before sent to H2 hpack compression to
  avoid capitalised H1 headers breaking H2 connections

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Separate hpack encoder and decoder for each ConnectionH2
- Filter out forbidden H2 headers in converter
- Remove kawa namespace prefix in pkawa

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Add BackendStatus in Position::Client, this will be used for
  reconnection and connection reuse
- Rename UpdateReadiness trait to Endpoint
- Add close method to Connection and Endpoint, for clients it handles
  the half closed streams
- Add end_stream method to Connection and Endpoint, for clients it
  handles connection reusability
- Add start_stream method to Connection and Endpoint, for clients it
  attaches a new stream
- Handle stream termination for H1 and H2 servers
- Router::connect tries to bundle and reuse connections (very basic
  scheme)
- Mark newly created clients as connecting (todo: handle success and
  failure appropriately)
- Implement Debug for Connection and its parts
- Force reponse length to 0 for HEAD requests in editor

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Create an H2 client connection if the cluster is tagged h2
- Write stream 0 first, make sure it's empty before continuing
- Mark closed H2 streams as inactive and reuse them
- Generate new increasing stream ideas (todo: make sure to "advertize"
  them in the right order)
- Fix response status parsing
- Temporary H1 compatibility fixes (should be fixed in Kawa):
  - Use "Default" as H2 response reason
  - Add "Content-Length: 0" on body less H2 messages

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Add expect_write to H2 connection to handle partial writes
- Centralize H2 socket writing in the same way as socket reading (with
  expect_read)
- Use expect_write for ClientPreface, ClientSettings, ServerSettings and
  Pong frames
- Try a very basic priority ordering (whith dummy priorities)

Other improvments:

- Add update_readiness_after_{read, write} to handle socket result and
  early return
- Fix H1 keep-alive by properly resetting Kawa and HttpContext
- Fix FrontTls read returning WouldBlock even when it shouldn't

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Add H2Error to CloseSession
- Call goaway upon CloseSession
- Replace many panics with CloseSession

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Added StreamState to Stream to merge active and token field as well as
  adding a "Link" variant to represent a Stream asking for connection
- Comment and implement start_stream and end_stream. In particular
  end_stream on a Server handles the error and reconnection logic
- Add set_default_answer which produces a default Kawa answer
- Add forcefully_terminate_answer which properly terminates H1 streams
  and sends RstStream on H2 streams
- Add connection loop with max retries and error handling in ready

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Primitive GoAway and connection error handling for H2 (todo: stream
  error handling)
- Add `force_disconnect` method to H2 that triggers the clean up routine
  by faking socket disconnection (experimental)
- Reading parts of a message properly inserts WRITABLE in the opposite
  endpoint readiness interest
- Fix stream_id numbering
- `set_default_answer` and `forcefully_terminate_answer` update StreamState
  to Unlinked
- Replace std::mem::swap with more appropriate variants
- Parameterize Mux with the Front type
- Replace kawa_h1 State by mux in HttpStateMachine
- Handle 1xx in Server ConnectionH1
- Add Upgrade to MuxResult to allow h1 to ws upgrade (implement
  upgrade_mux in HTTP Session)
- Implement Mux::shutting_down
- Handle https redirection in mux::Router::connect
- Implement a working 301 default answer
- Use the new default answer factory in e2e tests
- Fix front/back readiness for h1<->h1 Mux connections

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Fix readiness (set opposite endpoint as writable, after a read)
- Fix timeouts (with force_disconnect)
- Fix flags and phase handling for h2 headers
- Add support for h2 trailers in pkawa

Signed-off-by: Eloi DEMOLIS <[email protected]>
Signed-off-by: Eloi DEMOLIS <[email protected]>
@Wonshtrum Wonshtrum force-pushed the mux_h1_h2_errors_and_retries branch from fd40346 to 4436540 Compare November 29, 2023 18:58
@Keksoj
Copy link
Member

Keksoj commented Jun 25, 2024

I think this is made obsolete by #1104

I suggest we close the Pull Request

@Keksoj
Copy link
Member

Keksoj commented Jul 17, 2024

this is superseded by #1104

@Keksoj Keksoj closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants