win5do commented on a change in pull request #1257:
URL: https://github.com/apache/dubbo-go/pull/1257#discussion_r651421610
##########
File path: config/graceful_shutdown_config.go
##########
@@ -54,7 +58,7 @@ type ShutdownConfig struct {
RejectRequest bool
// true -> all requests had been processed. In provider side it means
that all requests are returned response to clients
// In consumer side, it means that all requests getting response from
servers
- RequestsFinished bool
+ RequestsFinished *atomic.Bool
Review comment:
RequestsFinished is pointer, can't be fill with config, requires
explicit assignment.
It's better to use value types.
##########
File path: filter/filter_impl/graceful_shutdown_filter.go
##########
@@ -61,7 +55,7 @@ func (gf *gracefulShutdownFilter) Invoke(ctx context.Context,
invoker protocol.I
}
atomic.AddInt32(&gf.activeCount, 1)
if gf.shutdownConfig != nil && gf.activeCount > 0 {
- gf.shutdownConfig.RequestsFinished = false
+ gf.shutdownConfig.RequestsFinished.Store(false)
Review comment:
Atomic use LOCK CPU instruction, may cause performance issues.
##########
File path: filter/filter_impl/graceful_shutdown_filter.go
##########
@@ -71,11 +65,24 @@ func (gf *gracefulShutdownFilter) OnResponse(ctx
context.Context, result protoco
atomic.AddInt32(&gf.activeCount, -1)
// although this isn't thread safe, it won't be a problem if the
gf.rejectNewRequest() is true.
Review comment:
This comment can be deleted now.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
