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

Add an example on how to setup timeouts #590

Closed
RyanGordon opened this issue Jul 14, 2017 · 5 comments
Closed

Add an example on how to setup timeouts #590

RyanGordon opened this issue Jul 14, 2017 · 5 comments
Assignees
Labels
documentation priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@RyanGordon
Copy link

RyanGordon commented Jul 14, 2017

I've been poring over the code looking for how to properly set timeouts / retries in the recommended way for the Spanner Client.

For example let's trace through the Spanner\Database::execute(..) function and how we could set timeouts for the request(s) it makes.

SpannerClient\Database::execute(..) has an $options array as the 2nd parameter but doesn't mention any timeout settings.

Next it calls Spanner\Operation::execute(..) which takes in the same $options array as 2nd parameter. Then it creates a callback which gets executed later but we'll skip to trace.

The options parameter is merged into the args parameter and the callback then calls: Spanner\Connection\Grpc::executeStreamingSql(..). This function then passes a callable [$this->spannerClient, 'executeStreamingSql'] and an array holding [session, sql, and the rest of the args] as the two parameters to Core\GrpcRequestWrapper::send(..) which takes 3 arguments but we don't pass in the third $options argument from the parent call which would have taken requestTimeout, retries, and grpcOptions. Now at this point the default retries/requestTimeout get merged into the grpcOptions defaults and then grpcOptions get merged into the last parameter of the $args parameter. This is then sent through to Spanner\v1\SpannerClient::executeStreamingSql(..) and the args array parameter is unwound to individual parameters by call_user_func_array

So presumably, assuming the number of arguments is always equal to 3, the last parameter will be optionalArgs at Spanner\v1\SpannerClient::executeStreamingSql(..) which is documented as explicitly taking timeoutMillis in the optionalArgs parameter but no mention of the other retrySettings, and timeoutMs parameters that could have been passed in from the previous function call.

So all in all it's somewhat confusing to understand where we should be setting these different timeout / retry parameters and what the recommended way would be. Just a couple of simple high level examples would be enough. I'd be happy to add some documentation if someone can clarify in this ticket :)

@RyanGordon
Copy link
Author

It would also be nice if we could replace/override configuration that came from the spanner_client_config.json file in some programmatic way. That would seem to be the cleanest way of customizing all of the different timeouts

@dwsupplee
Copy link
Contributor

@RyanGordon, thanks for creating this issue 👍. It's the kick in the butt we needed. We'll get this taken care of very soon :).

@dwsupplee dwsupplee added documentation priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Jul 14, 2017
@dwsupplee
Copy link
Contributor

@RyanGordon We are still working on getting documentation together (it's a little tricky, as we have been trying to normalize settings between REST/gRPC clients), but I just wanted to follow up and share a sample as to how you can currently configure retry settings programmatically.

require 'vendor/autoload.php';

use Google\Cloud\Spanner\SpannerClient;
use Google\GAX\RetrySettings;
use Google\GAX\BackoffSettings;

$spanner = new SpannerClient();
$db = $spanner->connect('instance', 'database');

$backoffSettings = new BackoffSettings([
    'initialRetryDelayMillis' => 1,
    'retryDelayMultiplier'    => 1,
    'maxRetryDelayMillis'     => 1,
    'initialRpcTimeoutMillis' => 1,
    'rpcTimeoutMultiplier'    => 1,
    'maxRpcTimeoutMillis'     => 1,
    'totalTimeoutMillis'      => 1
]);
$retryableCodes = [
    // for more codes, please see https://github.com/googleapis/gax-php/blob/master/src/GrpcConstants.php#L55-L73
    \Grpc\STATUS_INTERNAL
];
$options = [
    'grpcOptions' => [
        'timeoutMillis' => 10000,
        'retrySettings' => new RetrySettings($retryableCodes, $backoffSettings)
    ]
];

$db->insert(
    'theTable',
    [
        'id' => 100,
        'title' => 'Hello World'
    ],
    $options
);

Hope it helps, if there's anything else I can help clarify let me know!

@RyanGordon
Copy link
Author

Hi @dwsupplee,

Thank you for the example! This is exactly what we needed :)

@jdpedrie jdpedrie added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Aug 14, 2017
@jdpedrie
Copy link
Contributor

added to the feature request wiki.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants