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

Allow more control over Quic channel send #775

Open
hkhalili-ocado opened this issue Jan 28, 2025 · 3 comments
Open

Allow more control over Quic channel send #775

hkhalili-ocado opened this issue Jan 28, 2025 · 3 comments

Comments

@hkhalili-ocado
Copy link

Currently connectionSendAndFlush() in QuicheQuicChannel is called directly from within the same class and from QuicheQuicStreamChannel (Unsafe). This does not give any control to the user in terms of flushing. Could we instead use parent().pipeline().flush()? This way the user can add a handler to the QuicChannel pipeline to modify flushing behaviour. If this is potentially acceptable I can start working on implementing this for review.

@h-khalili
Copy link

Hi @normanmaurer. Any thoughts on this? Generally it looks like this implementation is not consistent with the Netty Framework's write/flush workflow. For example, in TCP writing to a channel simply buffers the data and does not flush it. Whereas writing to a QUIC stream channel, flushes the data. If you agree that the code needs to aligned with Netty, I would be happy to put together a few design proposals before I work on this?

@normanmaurer
Copy link
Member

@hkhalili-ocado I am happy to review a PR.

@h-khalili
Copy link

@normanmaurer That's great; please find below some thoughts.

Current behaviour

QuicStreamChannel

  • write: write to quiche (quiche_conn_stream_send) + QuicChannel::connectionSendAndFlush (quiche_conn_send + Datagram channel flush)
  • flush: NOOP

QuicChannel

  • write: datagram written to channelOutboundBuffer
  • flush: write datagram to quiche (quiche_conn_dgram_send) + QuicChannel::connectionSendAndFlush (if dgram exists)

Proposal for QuicChannel

  • write: // unchanged
  • flush: write datagram to quiche (quiche_conn_dgram_send) + QuicChannel::connectionSendAndFlush (if dgram data or stream data exists)

Proposal 1 for QuicStreamChannel

  • write: store in channelOutboundBuffer
  • flush: write to quiche (quiche_conn_stream_send)

Proposal 2 for QuicStreamChannel

  • write: write to quiche (quiche_conn_stream_send)
  • flush: QuicChannel::flush

Proposal 3 for QuicStreamChannel

  • write: write to quiche (quiche_conn_stream_send)
  • flush: throw Unimplemented exception (user would have to call flush on QuicChannel)

I'm leaning towards proposal 2. Do you have a preference for any of them or a 4th solution?

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

No branches or pull requests

3 participants