You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "marsevilspirit (via GitHub)" <gi...@apache.org> on 2025/08/17 10:26:06 UTC
[PR] perf(triple): use errgroup to handle graceful stop errors [dubbo-go]
marsevilspirit opened a new pull request, #2973:
URL: https://github.com/apache/dubbo-go/pull/2973
### Overview
This PR refactors the `Stop` and `GracefulStop` methods of the Triple protocol server. It replaces the previous manual goroutine management and error collection logic with `golang.org/x/sync/errgroup`, enhancing the code's simplicity, consistency, and maintainability.
### Key Improvements
#### 1\. **Unified Error Handling Pattern**
- **Before**: `Stop` and `GracefulStop` used different error handling approaches.
- **Now**: Both methods use a unified `errgroup` pattern, making the code more consistent.
#### 2\. **Simplified Concurrency Management**
- **Before**: Required manual management of `sync.WaitGroup`, error channels, and error collection.
- **Now**: Uses `eg.Go()` and `eg.Wait()` to automatically handle the goroutine lifecycle.
#### 3\. **Improved Context Propagation**
- **GracefulStop**: Correctly propagates the context, supporting timeouts and cancellations.
- **Stop**: Uses `context.Background()`, which is suitable for immediate shutdown scenarios.
#### 4\. **Code Cleanup**
- Removed unused imports for the `errors` and `strings` packages.
- Simplified the logic for error collection and formatting.
### Technical Implementation
#### **Before Refactor (Stop Method)**
```go
func (s *Server) Stop() error {
var errs []error
if s.httpSrv != nil {
if err := s.httpSrv.Close(); err != nil {
errs = append(errs, fmt.Errorf("http server close failed: %w", err))
}
}
if s.http3Srv != nil {
if err := s.http3Srv.Close(); err != nil {
errs = append(errs, fmt.Errorf("http3 server close failed: %w", err))
}
}
switch len(errs) {
case 0:
return nil
case 1:
return errs[0]
default:
var sb strings.Builder
sb.WriteString("multiple errors occurred during stop:")
for _, err := range errs {
sb.WriteString("\n\t- ")
sb.WriteString(err.Error())
}
return errors.New(sb.String())
}
}
```
#### **After Refactor (Stop Method)**
```go
func (s *Server) Stop() error {
eg, _ := errgroup.WithContext(context.Background())
if s.httpSrv != nil {
eg.Go(func() error {
if err := s.httpSrv.Close(); err != nil {
return fmt.Errorf("http server close failed: %w", err)
}
return nil
})
}
if s.http3Srv != nil {
eg.Go(func() error {
if err := s.http3Srv.Close(); err != nil {
return fmt.Errorf("http3 server close failed: %w", err)
}
return nil
})
}
return eg.Wait()
}
```
#### **Before Refactor (GracefulStop Method)**
```go
func (s *Server) GracefulStop(ctx context.Context) error {
var (
wg sync.WaitGroup
errChan = make(chan error, 2)
)
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)
}
}()
}
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)
var errs []error
for err := range errChan {
errs = append(errs, err)
}
switch len(errs) {
case 0:
return nil
case 1:
return errs[0]
default:
var sb strings.Builder
sb.WriteString("multiple errors occurred during graceful stop:")
for _, err := range errs {
sb.WriteString("\n\t- ")
sb.WriteString(err.Error())
}
return errors.New(sb.String())
}
}
```
#### **After Refactor (GracefulStop Method)**
```go
func (s *Server) GracefulStop(ctx context.Context) error {
eg, ctx := errgroup.WithContext(ctx)
if s.httpSrv != nil {
eg.Go(func() error {
if err := s.httpSrv.Shutdown(ctx); err != nil {
return fmt.Errorf("http server shutdown failed: %w", err)
}
return nil
})
}
if s.http3Srv != nil {
eg.Go(func() error {
if err := s.http3Srv.Shutdown(ctx); err != nil {
return fmt.Errorf("http3 server shutdown failed: %w", err)
}
return nil
})
}
return eg.Wait()
}
```
### Analysis of Advantages
#### **Improved Code Quality**
- **Readability**: The code is cleaner and the logic is clearer.
- **Consistency**: Both methods now use the same error handling pattern.
- **Maintainability**: Reduces duplicated code and unifies error handling logic.
#### **Concurrency Improvements**
- **Concurrent Shutdown**: The two servers can now be shut down in true concurrency.
- **Error Collection**: Automatically collects and consolidates errors from multiple goroutines.
- **Resource Management**: Better management of the goroutine lifecycle.
#### **Functional Enhancements**
- **Context Support**: `GracefulStop` now correctly supports context timeouts and cancellations.
- **Error Propagation**: Automatically handles error propagation and consolidation.
- **Concurrency Safety**: Provides built-in guarantees for concurrency safety.
### 📚 Related Documentation
- [[errgroup Package Documentation](https://pkg.go.dev/golang.org/x/sync/errgroup)](https://pkg.go.dev/golang.org/x/sync/errgroup)
- [Effective Go: Concurrency](https://golang.org/doc/effective_go#concurrency)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(triple): use errgroup to handle graceful stop errors [dubbo-go]
Posted by "marsevilspirit (via GitHub)" <gi...@apache.org>.
marsevilspirit commented on PR #2973:
URL: https://github.com/apache/dubbo-go/pull/2973#issuecomment-3218668338
I think this pr is ready to be merged.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(triple): use errgroup to handle graceful stop errors [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2973:
URL: https://github.com/apache/dubbo-go/pull/2973#issuecomment-3194293254
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=2973) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2973&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2973&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2973&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2973&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2973&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=2973)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(triple): use errgroup to handle graceful stop errors [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks merged PR #2973:
URL: https://github.com/apache/dubbo-go/pull/2973
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(triple): use errgroup to handle graceful stop errors [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2973:
URL: https://github.com/apache/dubbo-go/pull/2973#issuecomment-3194292219
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=2973) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2973&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2973&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2973&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2973&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2973&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=2973)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(triple): use errgroup to handle graceful stop errors [dubbo-go]
Posted by "marsevilspirit (via GitHub)" <gi...@apache.org>.
marsevilspirit commented on PR #2973:
URL: https://github.com/apache/dubbo-go/pull/2973#issuecomment-3196987022
> If new dependencies are introduced, check whether the concurrency control logic in other places can also be modified to maximize the benefits.
Sounds good. I will check it later.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org