dcelasun commented on a change in pull request #2497: URL: https://github.com/apache/thrift/pull/2497#discussion_r794008090
########## File path: lib/go/README.md ########## @@ -132,3 +132,28 @@ if this interval is set to a value too low (for example, 1ms), it might cause excessive cpu overhead. This feature is also only enabled on non-oneway endpoints. + +A note about server stop implementations +======================================== + +[TSimpleServer.Stop](https://pkg.go.dev/github.com/apache/thrift/lib/go/thrift#TSimpleServer.Stop) will wait for all client connections to be closed after +the last received or to be received request to be handled, so the time spent by stop + may be too long in some situation: +* when socket timeout is not set,server will be hanged before all active + clients to finish handling the last received or to be received request. +* when the socket timeout is too long saying one hour, server will be + hanged for one hour before all active clients to finish handling the + last received or to be received request. Review comment: ```suggestion * When the socket timeout is too long (e.g one hour), server will hang for that duration before all active clients finish handling the last received received request. ``` ########## File path: lib/go/README.md ########## @@ -132,3 +132,28 @@ if this interval is set to a value too low (for example, 1ms), it might cause excessive cpu overhead. This feature is also only enabled on non-oneway endpoints. + +A note about server stop implementations +======================================== + +[TSimpleServer.Stop](https://pkg.go.dev/github.com/apache/thrift/lib/go/thrift#TSimpleServer.Stop) will wait for all client connections to be closed after +the last received or to be received request to be handled, so the time spent by stop Review comment: ```suggestion the last received request to be handled, so the time spent by stop ``` ########## File path: lib/go/README.md ########## @@ -132,3 +132,28 @@ if this interval is set to a value too low (for example, 1ms), it might cause excessive cpu overhead. This feature is also only enabled on non-oneway endpoints. + +A note about server stop implementations +======================================== + +[TSimpleServer.Stop](https://pkg.go.dev/github.com/apache/thrift/lib/go/thrift#TSimpleServer.Stop) will wait for all client connections to be closed after +the last received or to be received request to be handled, so the time spent by stop + may be too long in some situation: Review comment: ```suggestion the last received request to be handled, as the time spent by Stop may sometimes be too long: ``` ########## File path: lib/go/README.md ########## @@ -132,3 +132,28 @@ if this interval is set to a value too low (for example, 1ms), it might cause excessive cpu overhead. This feature is also only enabled on non-oneway endpoints. + +A note about server stop implementations +======================================== + +[TSimpleServer.Stop](https://pkg.go.dev/github.com/apache/thrift/lib/go/thrift#TSimpleServer.Stop) will wait for all client connections to be closed after +the last received or to be received request to be handled, so the time spent by stop + may be too long in some situation: +* when socket timeout is not set,server will be hanged before all active + clients to finish handling the last received or to be received request. +* when the socket timeout is too long saying one hour, server will be + hanged for one hour before all active clients to finish handling the + last received or to be received request. + +To avoid the server stop to be hanged for too long, you can take +advantage of thrift.ServerStopTimeout in your main or init function: + + thrift.ServerStopTimeout = <max_duration_to_stop> + +If it's set to <=0, the feature will be disabled(by default), and server +will wait for all the client connections to be closed gracefully with +zero err time.Otherwise, the stop will wait for all the client +connections to be closed gracefully util thrift.ServerStopTimeout is +reached, and the client connections that are not closed after thrift. Review comment: ```suggestion reached, and client connections that are not closed after thrift. ``` ########## File path: lib/go/README.md ########## @@ -132,3 +132,28 @@ if this interval is set to a value too low (for example, 1ms), it might cause excessive cpu overhead. This feature is also only enabled on non-oneway endpoints. + +A note about server stop implementations +======================================== + +[TSimpleServer.Stop](https://pkg.go.dev/github.com/apache/thrift/lib/go/thrift#TSimpleServer.Stop) will wait for all client connections to be closed after +the last received or to be received request to be handled, so the time spent by stop + may be too long in some situation: +* when socket timeout is not set,server will be hanged before all active + clients to finish handling the last received or to be received request. +* when the socket timeout is too long saying one hour, server will be + hanged for one hour before all active clients to finish handling the + last received or to be received request. + +To avoid the server stop to be hanged for too long, you can take +advantage of thrift.ServerStopTimeout in your main or init function: Review comment: ```suggestion To prevent Stop from hanging for too long, you can set thrift.ServerStopTimeout in your main or init function: ``` ########## File path: lib/go/README.md ########## @@ -132,3 +132,28 @@ if this interval is set to a value too low (for example, 1ms), it might cause excessive cpu overhead. This feature is also only enabled on non-oneway endpoints. + +A note about server stop implementations +======================================== + +[TSimpleServer.Stop](https://pkg.go.dev/github.com/apache/thrift/lib/go/thrift#TSimpleServer.Stop) will wait for all client connections to be closed after +the last received or to be received request to be handled, so the time spent by stop + may be too long in some situation: +* when socket timeout is not set,server will be hanged before all active + clients to finish handling the last received or to be received request. +* when the socket timeout is too long saying one hour, server will be + hanged for one hour before all active clients to finish handling the + last received or to be received request. + +To avoid the server stop to be hanged for too long, you can take +advantage of thrift.ServerStopTimeout in your main or init function: + + thrift.ServerStopTimeout = <max_duration_to_stop> + +If it's set to <=0, the feature will be disabled(by default), and server +will wait for all the client connections to be closed gracefully with +zero err time.Otherwise, the stop will wait for all the client Review comment: What does `with zero err time` mean 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]
