✨ enable grpc keepalive. #70
Conversation
morvencao
force-pushed
the
br_enable_grpc_keepalive
branch
from
127f758 to
a512ec8
Compare
|
/assign @qiujian16 @skeeey |
|
|
||
| // PermitWithoutStream determines if keepalive pings are sent when there are no active RPCs. | ||
| // If false, pings are not sent and Time and Timeout are ignored. Default is false. | ||
| PermitWithoutStream bool `json:"permitWithoutStream,omitempty" yaml:"permitWithoutStream,omitempty"` |
There was a problem hiding this comment.
what is the different with the enable? if we set enable=true and permitwithoutstream=false, what happens?
There was a problem hiding this comment.
enable is the global flag to control whether enable keepalive or not, while permitwithoutstream is grpc keepalive parameter to control whether the client should send keepalive pings even with no active RPCs, see: https://pkg.go.dev/google.golang.org/grpc/keepalive#ClientParameters
if we set enable=true and permitwithoutstream=false, the keep alive will be enabled, but it will not send ping if no rpc streaming (for example, no client is subscribing to resource status using grpc stream).
There was a problem hiding this comment.
so we may enable this by default? because we generally need a subscription
There was a problem hiding this comment.
Will raise another PR in maestro to enable this.
| type KeepAliveConfig struct { | ||
| // Enable specifies whether the keepalive option is enabled. | ||
| // When disabled, other keepalive configurations are ignored. Default is false. | ||
| Enable bool `json:"enable,omitempty" yaml:"enable,omitempty"` |
There was a problem hiding this comment.
Under what cases should we enable this?
There was a problem hiding this comment.
If we want to quick detect the broken connection (for example, the broken connection caused by broken network), we can enable this.
When we enable the keepalive, client will send keeepalive pings with configured interval, if we can't receive ping response for a certain duration, the client will close the connection (state from READY to IDLE), and we have logic to recreate the cloudevents client once we detect the connection become IDLE.
There was a problem hiding this comment.
so for our case, we may enable this by default?
There was a problem hiding this comment.
Will raise another PR in maestro to enable this.
morvencao
force-pushed
the
br_enable_grpc_keepalive
branch
from
a512ec8 to
37faf51
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: morvencao The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: morvencao <[email protected]>
morvencao
force-pushed
the
br_enable_grpc_keepalive
branch
from
37faf51 to
3937d37
Compare
|
/assign @qiujian16 |
Summary
This PR introduces keepalive parameters to the cloudevents client using gRPC. By default, these parameters are disabled. Users can optionally enable it to detect broken gRPC connections. MQTT has similar feature to detect the half-open connection.
Related issue(s)
Fixes #