search

googleapis / sdk-platform-java

Tooling and shared libraries for Cloud SDK for Java
https://cloud.google.com/java/docs/bom
Apache License 2.0
65 stars 53 forks source link

feat: make stream wait timeout a first class citizen #1569

Closed mutianf closed 1 year ago

mutianf commented 1 year ago

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Is your feature request related to a problem? Please describe. For a server streaming api, there are 4 conceptual timeouts:

  1. overall operation timeout - the maximum amount time that passes from the user invoking a method until that method exits
  2. attempt/rpc timeout - if retries are enabled the maximum amount of time that passes for each attempt in an operation
  3. message wait timeout - the maximum amount of time to wait for the next message from the server
  4. idle timeout - how long to wait before considering the stream orphaned by the user and closing it

Each has a usecase:

  1. operation timeout is useful for users to fulfill their own slo guarantees
  2. attempt timeout are useful when a client developer knows the absolute limit of an rpc but that limit happens to be shorter than the required slo for a customer. For example a point read of a bigtable key shouldnt ever take more than 100ms. If it does, then we can assume something is wrong (GFE died without sending a FIN packet) and abort the request and retry.
  3. message wait timeouts have a similar use as attempt timeouts with tighter guarantees
  4. idle timeouts are useful to reduce buffer bloat on the server Currently all of them are implemented in gax but the delineation was muddied by me a while back. This PR tries to fix the situation. In the current world, operation timeout is defined by RetrySettings#totalTimeout, idle timeout is defined by ServerStreamingCallSettings#idleTimeout. However RetrySettings#rpcTimeout is mapped to message wait timeout and attempt timeout is only configureable per call using ApiCallContext#withTimeout.

Describe the solution you'd like https://github.com/googleapis/gapic-generator-java/pull/1473 but backward compatible

blakeli0 commented 1 year ago

Fixed by https://github.com/googleapis/sdk-platform-java/pull/1473