Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RetrySettings for StreamingRead/StreamingSql #2320

Open
dominity opened this issue Mar 6, 2023 · 1 comment
Open

RetrySettings for StreamingRead/StreamingSql #2320

dominity opened this issue Mar 6, 2023 · 1 comment
Assignees
Labels
api: spanner Issues related to the googleapis/java-spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@dominity
Copy link

dominity commented Mar 6, 2023

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

Is your feature request related to a problem? Please describe.
Recently I was working on fine tunning timeouts/retry settings for Spanner requests for one of applications.
I noticed that initRpcTimeout doesn't works on StreamingRead/StreamingSql type of requests.
So I started to go through source code and found that there are 3 places/configurations where I can work with timeouts:

  • streamWaitTimeout property from GrpcCallContext which is used by WatchdogServerStreamingCallable to interrupt streaming request
  • timeout property from GrpcCallContext which is used as source for 'grpc-timeout' header (if specified, otherwise totalTimeout from retrySettings is used)
  • streamWatchdogCheckInterval from StubSettings which defines check interval for Watchdog used to interrupt streaming

First two aren't configurable from RetrySettings level.
Could you please makes them part of RetrySettings, so I don't have to interact with GrpcCallContext directly?

Describe the solution you'd like
Either to have separate set of properties in RetrySettings like (and recalculate them similarly to initRpcTimeout during retries):

var settings = RetrySettings.newBuilder()
                    .setStreamWaitTimeout(Duration.ofMillis(streamWaitTimeout))
                    .setSingleRequestTimeout(Duration.ofMillis(singleRequestTimeout)) // used as grpc-timeout

or reuse initRpcTimeout as streamWaitTimeout/grpc-timeout:

var settings = RetrySettings.newBuilder()
                    .setInitialRpcTimeout(Duration.ofMillis(initialRpcTimeout)) // used as both streamingWaitTimeout and grpc-timeout

Describe alternatives you've considered
Currently I interact with GrpcCallContext directly to set this values:

    public static class SpannerGrpcCallContextConfigurator implements SpannerOptions.CallContextConfigurator {
        public <ReqT, RespT> ApiCallContext configure(ApiCallContext context, ReqT request,
                MethodDescriptor<ReqT, RespT> method) {
            if (method == SpannerGrpc.getStreamingReadMethod()) {
                return GrpcCallContext.createDefault().
                        .withTimeout(/*some timeout*/)
                        .withStreamWaitTimeout(/*some timeout*/);
            }
            return null;
        }
    }
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Mar 6, 2023
@ansh0l ansh0l assigned rahul2393 and unassigned ansh0l Mar 8, 2023
@rajatbhatta rajatbhatta added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Aug 12, 2023
@rajatbhatta rajatbhatta assigned arpan14 and unassigned rahul2393 and rajatbhatta Aug 12, 2023
@rajatbhatta
Copy link
Contributor

This is a feature request. Assigning to @arpan14 to take it forward.

@arpan14 arpan14 assigned rahul2393 and unassigned arpan14 Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

5 participants