diff --git a/CHANGES.md b/CHANGES.md index 65ed07f5066..c4b222326a4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -28,7 +28,7 @@ - [THRIFT-5164](https://issues.apache.org/jira/browse/THRIFT-5164) - Add ClientMiddleware function type and WrapClient function to support wrapping a TClient with middleware functions. - [THRIFT-5164](https://issues.apache.org/jira/browse/THRIFT-5164) - Add ProcessorMiddleware function type and WrapProcessor function to support wrapping a TProcessor with middleware functions. - [THRIFT-5233](https://issues.apache.org/jira/browse/THRIFT-5233) - Add context deadline check to ReadMessageBegin in TBinaryProtocol, TCompactProtocol, and THeaderProtocol. -- [THRIFT-5240](https://issues.apache.org/jira/browse/THRIFT-5240) - The context passed into server handler implementations will be canceled when we detected that the client closed the connection. +- [THRIFT-5240](https://issues.apache.org/jira/browse/THRIFT-5240) - The context passed into server handler implementations will be canceled when we detected that the client closed the connection (This feature is disabled by default due to a go runtime bug. Check [go library README](lib/go/README.md) on more details and how to enable it.) ## 0.13.0 diff --git a/lib/go/README.md b/lib/go/README.md index 5b7e2cd464e..2085932fc9f 100644 --- a/lib/go/README.md +++ b/lib/go/README.md @@ -85,12 +85,21 @@ which will generate: A note about server handler implementations =========================================== -The context object passed into the server handler function will be canceled when -the client closes the connection (this is a best effort check, not a guarantee --- there's no guarantee that the context object is always canceled when client -closes the connection, but when it's canceled you can always assume the client -closed the connection). When implementing Go Thrift server, you can take -advantage of that to abandon requests that's no longer needed: +The thrift compiler compiled go code has the ability to check connectivity with +a ticker and cancel the context object passed into the server handler function +when it detects the client side closed the connection (this is a best effort +check, not a guarantee -- there's no guarantee that the context object is always +canceled when client closes the connection, but when it's canceled you can +always assume the client closed the connection). By default this feature is +disabled due to a Go runtime [bug](https://github.com/golang/go/issues/27707) +that could cause excessive CPU usage. If you want to enable this feature, set +the ticker interval to a positive value early in your main function: + + // Enable server side connectivity check with 1ms ticker interval + thrift.ServerConnectivityCheckInterval = time.Millisecond + +After enabled it, when implementing Go Thrift server, you can take advantage of +that to abandon requests that's no longer needed: func MyEndpoint(ctx context.Context, req *thriftRequestType) (*thriftResponseType, error) { ... @@ -100,11 +109,4 @@ advantage of that to abandon requests that's no longer needed: ... } -This feature would add roughly 1 millisecond of latency overhead to the server -handlers (along with roughly 2 goroutines per request). -If that is unacceptable, it can be disabled by having this line early in your -main function: - - thrift.ServerConnectivityCheckInterval = 0 - -This feature is also only enabled on non-oneway endpoints. +This feature is also only available on non-oneway endpoints. diff --git a/lib/go/thrift/simple_server.go b/lib/go/thrift/simple_server.go index 68ac394c6c2..1c015cc7b4e 100644 --- a/lib/go/thrift/simple_server.go +++ b/lib/go/thrift/simple_server.go @@ -45,8 +45,16 @@ var ErrAbandonRequest = errors.New("request abandoned") // It's defined as a variable instead of constant, so that thrift server // implementations can change its value to control the behavior. // -// If it's changed to <=0, the feature will be disabled. -var ServerConnectivityCheckInterval = time.Millisecond +// The feature is disabled when it's <=0. Set it to a positive value early in +// your main function to enable this feature, e.g.: +// +// func main() { +// // Enable server connectivity check with 1ms interval ticker. +// thrift.ServerConnectivityCheckInterval = time.Millisecond +// +// // ... other main function code +// } +var ServerConnectivityCheckInterval time.Duration = 0 /* * This is not a typical TSimpleServer as it is not blocked after accept a socket.