buptubuntu commented on a change in pull request #2497:
URL: https://github.com/apache/thrift/pull/2497#discussion_r779993940
##########
File path: lib/go/thrift/simple_server.go
##########
@@ -193,7 +195,13 @@ func (p *TSimpleServer) innerAccept() (int32, error) {
}
if client != nil {
p.wg.Add(1)
+ p.clients[client] = struct{}{}
go func() {
+ defer func() {
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ delete(p.clients, client)
Review comment:
I have also considered to add one additional goroutine per client
connection，but not sure if that will cause performance problem. But if we want
to fix the map key issue we will have to make big change to TTrancsport， that
will be another issue that we will handle in the future.Besides the cost of the
additinal go routine should be quite small comparing to the cost of user
defined processor or the data read and write operations in transport.
So I think this is a better solution theoretically
I will try to get the cost of the additional goroutine for some extreme
cases:
1>1000~10000 client connect to server but not send data for a long time
2>1000~10000 client connect to server and send 1k data per second
--
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]
