-
Notifications
You must be signed in to change notification settings - Fork 483
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
feat: add QUIC server support #423
base: main
Are you sure you want to change the base?
Conversation
… with timeout support
- Modified GetAddr, GetConnChan - Added EnableProxyProtocol, IsRunning, GetConfiguredAddress, ListenAndServe - Updated Read, Write methods - Implemented GetNextMessage - Removed SetDeadline methods
- Modified GetAddr, GetConnChan - Added EnableProxyProtocol, IsRunning, GetConfiguredAddress, ListenAndServe - Updated Read, Write methods - Implemented GetNextMessage - Removed SetDeadline methods
…taya into feature/quic-server
} | ||
|
||
// EnableProxyProtocol not implemented for QUIC, keep as No-op or implement as needed | ||
func (a *QuicAcceptor) EnableProxyProtocol() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@guthyerrz I think it's important to understand who's using proxy protocol and what's the use case, so we evaluate if we need to implement this right away or not
pkg/acceptor/quic_acceptor.go
Outdated
|
||
// Write writes data to the connection with a defined deadline | ||
func (q *QuicConnWrapper) Write(p []byte) (int, error) { | ||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) // 5 seconds timeout as an example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this timeout configurable instead of hardcoding it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do it
|
||
// Close closes the QUIC connection | ||
func (q *QuicConnWrapper) Close() error { | ||
return q.conn.CloseWithError(0, "closed") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't really read the API here, but do all connections need to be closed with errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quic-go documentation specifies that to close a connection you must use CloseWithError, as there is no simple Close() method. quic-go does not provide a way to close the connection without reporting an error code and message
pkg/acceptor/quic_acceptor.go
Outdated
|
||
// GetNextMessage reads the next message available in the QUIC stream | ||
func (q *QuicConnWrapper) GetNextMessage() (b []byte, err error) { | ||
// Read data from the stream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't you leverage io utilities that go provides in this method, just like we do for the tcp acceptor?
https://github.com/topfreegames/pitaya/blob/main/pkg/acceptor/tcp_acceptor.go#L55-L74
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, you seem to be assuming the maximum size of a message will be 1024 bytes, which is inaccurate
…imeout, and error handling
This change introduces a new `--acceptor` flag, allowing users to choose between TCP and QUIC protocols when running the server. Examples: - For TCP: `go run main.go --acceptor=tcp` - For QUIC: `go run main.go --acceptor=quic`
Enable support for QUIC protocol to expand connection options in supported environments.