No-SilverBullet commented on code in PR #2916:
URL: https://github.com/apache/dubbo-go/pull/2916#discussion_r2154699391
##########
protocol/triple/triple_protocol/server.go:
##########
@@ -160,44 +172,161 @@ func (s *Server) RegisterCompatStreamHandler(
return nil
}
-func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
- handler, pattern := s.mux.Handler(r)
- if pattern == "" {
- logger.Warnf("404: didn't register this method - %s\n",
r.URL.Path)
+func (s *Server) Run(callProtocol string, tlsConf *tls.Config) error {
+ // TODO: Refactor to support starting HTTP/2 and HTTP/3 servers
simultaneously.
+ // The current switch logic is mutually exclusive. Future work should
allow enabling
+ // both protocols, likely based on configuration, and run them
concurrently.
+ switch callProtocol {
+ case constant.CallHTTP2:
+ return s.startHttp2(tlsConf)
+ case constant.CallHTTP3:
+ return s.startHttp3(tlsConf)
+ default:
+ return fmt.Errorf("unsupported protocol: %s, only http2 or
http3 are supported", callProtocol)
}
-
- handler.ServeHTTP(w, r)
}
-func (s *Server) Run() error {
- s.httpSrv.Handler = h2c.NewHandler(s, &http2.Server{})
+func (s *Server) startHttp2(tlsConf *tls.Config) error {
+ s.httpSrv = &http.Server{
+ Addr: s.addr,
+ Handler: h2c.NewHandler(s.mux, &http2.Server{}),
+ TLSConfig: tlsConf,
+ }
+
+ logger.Debugf("TRIPLE HTTP/2 Server starting on %v", s.addr)
var err error
- if s.httpSrv.TLSConfig != nil {
- // TODO: Maybe we should be able to find a better way to start
TLS.
+
+ if tlsConf != nil {
err = s.httpSrv.ListenAndServeTLS("", "")
} else {
err = s.httpSrv.ListenAndServe()
}
+
return err
}
-func (s *Server) SetTLSConfig(c *tls.Config) {
- s.httpSrv.TLSConfig = c
+func (s *Server) startHttp3(tlsConf *tls.Config) error {
+ if tlsConf == nil {
+ return fmt.Errorf("TRIPLE HTTP/3 Server must have TLS config,
but TLS config is nil")
+ }
+
+ s.http3Srv = &http3.Server{
+ Addr: s.addr,
+ Handler: s.mux,
+ // Adapt and enhance a generic tls.Config object into a
configuration
+ // specifically for HTTP/3 services.
+ // ref:
https://quic-go.net/docs/http3/server/#setting-up-a-http3server
+ TLSConfig: http3.ConfigureTLSConfig(tlsConf),
+ // TODO: Detailed QUIC configuration.
+ QUICConfig: &quic.Config{},
+ }
+
+ logger.Debugf("TRIPLE HTTP/3 Server starting on %v", s.addr)
+
+ return s.http3Srv.ListenAndServe()
}
+// Stop the Triple server for both HTTP/2 and HTTP/3.
+// Because stop is very fast, there is no need to parallelize stop.
func (s *Server) Stop() error {
- return s.httpSrv.Close()
+ var errs []error
+
+ // stop HTTP server
+ if s.httpSrv != nil {
+ if err := s.httpSrv.Close(); err != nil {
+ errs = append(errs, fmt.Errorf("http server close
failed: %w", err))
+ }
+ }
+
+ // stop HTTP/3 server
+ if s.http3Srv != nil {
+ if err := s.http3Srv.Close(); err != nil {
+ errs = append(errs, fmt.Errorf("http3 server close
failed: %w", err))
+ }
+ }
+
+ if len(errs) == 0 {
+ return nil
+ }
+
+ if len(errs) == 1 {
+ return errs[0]
+ }
+
+ var sb strings.Builder
+ sb.WriteString("multiple errors occurred during stop:")
+ for _, err := range errs {
+ // Newline and indent for easier reading
+ sb.WriteString("\n\t- ")
+ sb.WriteString(err.Error())
+ }
+ return errors.New(sb.String())
}
+// Gracefulstop shutdown the Triple server for both HTTP/2 and HTTP/3
gracefully.
+// Because graceful shutdown is slow, I adopted concurrent processing.
func (s *Server) GracefulStop(ctx context.Context) error {
- return s.httpSrv.Shutdown(ctx)
+ var (
+ wg sync.WaitGroup
+ errChan = make(chan error, 2)
+ )
+
+ // shutdown HTTP server
+ if s.httpSrv != nil {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ if err := s.httpSrv.Shutdown(ctx); err != nil {
+ errChan <- fmt.Errorf("http server shutdown
failed: %w", err)
+ }
+ }()
+ }
+
+ // shutdown HTTP/3 server
+ if s.http3Srv != nil {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ if err := s.http3Srv.Shutdown(ctx); err != nil {
+ errChan <- fmt.Errorf("http3 server shutdown
failed: %w", err)
+ }
+ }()
+ }
+
+ wg.Wait()
+ close(errChan)
+
+ // Error Collection and Handling.
+ // Collect all errors into a slice.
+ var errs []error
+ for err := range errChan {
+ errs = append(errs, err)
+ }
+
+ if len(errs) == 0 {
+ return nil
+ }
+
+ if len(errs) == 1 {
+ return errs[0]
+ }
+
+ var sb strings.Builder
+ sb.WriteString("multiple errors occurred during graceful stop:")
+ for _, err := range errs {
+ // Newline and indent for easier reading
+ sb.WriteString("\n\t- ")
+ sb.WriteString(err.Error())
+ }
+
+ return errors.New(sb.String())
Review Comment:
i think use switch may be more clear
##########
protocol/triple/triple_protocol/server.go:
##########
@@ -160,44 +172,161 @@ func (s *Server) RegisterCompatStreamHandler(
return nil
}
-func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
- handler, pattern := s.mux.Handler(r)
- if pattern == "" {
- logger.Warnf("404: didn't register this method - %s\n",
r.URL.Path)
+func (s *Server) Run(callProtocol string, tlsConf *tls.Config) error {
+ // TODO: Refactor to support starting HTTP/2 and HTTP/3 servers
simultaneously.
+ // The current switch logic is mutually exclusive. Future work should
allow enabling
+ // both protocols, likely based on configuration, and run them
concurrently.
+ switch callProtocol {
+ case constant.CallHTTP2:
+ return s.startHttp2(tlsConf)
+ case constant.CallHTTP3:
+ return s.startHttp3(tlsConf)
+ default:
+ return fmt.Errorf("unsupported protocol: %s, only http2 or
http3 are supported", callProtocol)
}
-
- handler.ServeHTTP(w, r)
}
-func (s *Server) Run() error {
- s.httpSrv.Handler = h2c.NewHandler(s, &http2.Server{})
+func (s *Server) startHttp2(tlsConf *tls.Config) error {
+ s.httpSrv = &http.Server{
+ Addr: s.addr,
+ Handler: h2c.NewHandler(s.mux, &http2.Server{}),
+ TLSConfig: tlsConf,
+ }
+
+ logger.Debugf("TRIPLE HTTP/2 Server starting on %v", s.addr)
var err error
- if s.httpSrv.TLSConfig != nil {
- // TODO: Maybe we should be able to find a better way to start
TLS.
+
+ if tlsConf != nil {
err = s.httpSrv.ListenAndServeTLS("", "")
} else {
err = s.httpSrv.ListenAndServe()
}
+
return err
}
-func (s *Server) SetTLSConfig(c *tls.Config) {
- s.httpSrv.TLSConfig = c
+func (s *Server) startHttp3(tlsConf *tls.Config) error {
+ if tlsConf == nil {
+ return fmt.Errorf("TRIPLE HTTP/3 Server must have TLS config,
but TLS config is nil")
+ }
+
+ s.http3Srv = &http3.Server{
+ Addr: s.addr,
+ Handler: s.mux,
+ // Adapt and enhance a generic tls.Config object into a
configuration
+ // specifically for HTTP/3 services.
+ // ref:
https://quic-go.net/docs/http3/server/#setting-up-a-http3server
+ TLSConfig: http3.ConfigureTLSConfig(tlsConf),
+ // TODO: Detailed QUIC configuration.
+ QUICConfig: &quic.Config{},
+ }
+
+ logger.Debugf("TRIPLE HTTP/3 Server starting on %v", s.addr)
+
+ return s.http3Srv.ListenAndServe()
}
+// Stop the Triple server for both HTTP/2 and HTTP/3.
+// Because stop is very fast, there is no need to parallelize stop.
func (s *Server) Stop() error {
- return s.httpSrv.Close()
+ var errs []error
+
+ // stop HTTP server
+ if s.httpSrv != nil {
+ if err := s.httpSrv.Close(); err != nil {
+ errs = append(errs, fmt.Errorf("http server close
failed: %w", err))
+ }
+ }
+
+ // stop HTTP/3 server
+ if s.http3Srv != nil {
+ if err := s.http3Srv.Close(); err != nil {
+ errs = append(errs, fmt.Errorf("http3 server close
failed: %w", err))
+ }
+ }
+
+ if len(errs) == 0 {
+ return nil
+ }
+
+ if len(errs) == 1 {
+ return errs[0]
+ }
+
+ var sb strings.Builder
+ sb.WriteString("multiple errors occurred during stop:")
+ for _, err := range errs {
+ // Newline and indent for easier reading
+ sb.WriteString("\n\t- ")
+ sb.WriteString(err.Error())
+ }
+ return errors.New(sb.String())
Review Comment:
i think use switch may be more clear
##########
protocol/triple/triple_protocol/server.go:
##########
@@ -160,44 +172,161 @@ func (s *Server) RegisterCompatStreamHandler(
return nil
}
-func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
- handler, pattern := s.mux.Handler(r)
- if pattern == "" {
- logger.Warnf("404: didn't register this method - %s\n",
r.URL.Path)
+func (s *Server) Run(callProtocol string, tlsConf *tls.Config) error {
+ // TODO: Refactor to support starting HTTP/2 and HTTP/3 servers
simultaneously.
+ // The current switch logic is mutually exclusive. Future work should
allow enabling
+ // both protocols, likely based on configuration, and run them
concurrently.
+ switch callProtocol {
+ case constant.CallHTTP2:
+ return s.startHttp2(tlsConf)
+ case constant.CallHTTP3:
+ return s.startHttp3(tlsConf)
+ default:
+ return fmt.Errorf("unsupported protocol: %s, only http2 or
http3 are supported", callProtocol)
}
-
- handler.ServeHTTP(w, r)
}
-func (s *Server) Run() error {
- s.httpSrv.Handler = h2c.NewHandler(s, &http2.Server{})
+func (s *Server) startHttp2(tlsConf *tls.Config) error {
+ s.httpSrv = &http.Server{
+ Addr: s.addr,
+ Handler: h2c.NewHandler(s.mux, &http2.Server{}),
+ TLSConfig: tlsConf,
+ }
+
+ logger.Debugf("TRIPLE HTTP/2 Server starting on %v", s.addr)
var err error
- if s.httpSrv.TLSConfig != nil {
- // TODO: Maybe we should be able to find a better way to start
TLS.
+
+ if tlsConf != nil {
err = s.httpSrv.ListenAndServeTLS("", "")
} else {
err = s.httpSrv.ListenAndServe()
}
+
return err
}
-func (s *Server) SetTLSConfig(c *tls.Config) {
- s.httpSrv.TLSConfig = c
+func (s *Server) startHttp3(tlsConf *tls.Config) error {
+ if tlsConf == nil {
+ return fmt.Errorf("TRIPLE HTTP/3 Server must have TLS config,
but TLS config is nil")
+ }
+
+ s.http3Srv = &http3.Server{
+ Addr: s.addr,
+ Handler: s.mux,
+ // Adapt and enhance a generic tls.Config object into a
configuration
+ // specifically for HTTP/3 services.
+ // ref:
https://quic-go.net/docs/http3/server/#setting-up-a-http3server
+ TLSConfig: http3.ConfigureTLSConfig(tlsConf),
+ // TODO: Detailed QUIC configuration.
+ QUICConfig: &quic.Config{},
+ }
+
+ logger.Debugf("TRIPLE HTTP/3 Server starting on %v", s.addr)
+
+ return s.http3Srv.ListenAndServe()
}
+// Stop the Triple server for both HTTP/2 and HTTP/3.
+// Because stop is very fast, there is no need to parallelize stop.
func (s *Server) Stop() error {
- return s.httpSrv.Close()
+ var errs []error
+
+ // stop HTTP server
+ if s.httpSrv != nil {
+ if err := s.httpSrv.Close(); err != nil {
+ errs = append(errs, fmt.Errorf("http server close
failed: %w", err))
+ }
+ }
+
+ // stop HTTP/3 server
+ if s.http3Srv != nil {
+ if err := s.http3Srv.Close(); err != nil {
+ errs = append(errs, fmt.Errorf("http3 server close
failed: %w", err))
+ }
+ }
+
+ if len(errs) == 0 {
+ return nil
+ }
+
+ if len(errs) == 1 {
+ return errs[0]
+ }
+
+ var sb strings.Builder
+ sb.WriteString("multiple errors occurred during stop:")
+ for _, err := range errs {
+ // Newline and indent for easier reading
+ sb.WriteString("\n\t- ")
+ sb.WriteString(err.Error())
+ }
+ return errors.New(sb.String())
}
+// Gracefulstop shutdown the Triple server for both HTTP/2 and HTTP/3
gracefully.
+// Because graceful shutdown is slow, I adopted concurrent processing.
func (s *Server) GracefulStop(ctx context.Context) error {
- return s.httpSrv.Shutdown(ctx)
+ var (
+ wg sync.WaitGroup
+ errChan = make(chan error, 2)
+ )
+
+ // shutdown HTTP server
+ if s.httpSrv != nil {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ if err := s.httpSrv.Shutdown(ctx); err != nil {
+ errChan <- fmt.Errorf("http server shutdown
failed: %w", err)
Review Comment:
If shutdown takes a long time or may cause blockage, timeout processing
should be added here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
