Skip to content

Commit

Permalink
THRIFT-5240: Disable go server side connectivity check by default
Browse files Browse the repository at this point in the history
Client: go

This is a follow up to 4db7a0a.

Due to the go runtime bug [1], the 1ms ticker used by this feature could
cause excessive cpu usage. So until the bug is fixed in go it's better
to leave this behavior disabled by default. The instructions in the
README file has been updated to how to enable this feature if needed.

[1] golang/go#27707
  • Loading branch information
fishy committed Oct 7, 2020
1 parent 6d57026 commit 6c6361d
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 17 deletions.
2 changes: 1 addition & 1 deletion CHANGES.md
Expand Up @@ -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

Expand Down
30 changes: 16 additions & 14 deletions lib/go/README.md
Expand Up @@ -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) {
...
Expand All @@ -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.
12 changes: 10 additions & 2 deletions lib/go/thrift/simple_server.go
Expand Up @@ -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.
Expand Down

0 comments on commit 6c6361d

Please sign in to comment.