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

fix(Spanner): add retry for RST_STREAM #5938

Merged
merged 10 commits into from
Apr 5, 2023
3 changes: 2 additions & 1 deletion .github/workflows/spanner-emulator-system-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ jobs:

- name: Install dependencies
run: |
composer config repositories.local --json '{"type":"path", "url": "../Core", "options": {"versions": {"google/cloud-core": "1.45"}}}' -d Spanner
# ensure composer uses local Core instead of pulling from packagist
composer config repositories.local --json '{"type":"path", "url": "../Core", "options": {"versions": {"google/cloud-core": "1.100"}}}' -d Spanner
composer update --prefer-dist --no-interaction --no-suggest -d Spanner/

- name: Run system tests
Expand Down
2 changes: 1 addition & 1 deletion Spanner/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"minimum-stability": "stable",
"require": {
"ext-grpc": "*",
"google/cloud-core": "^1.43",
"google/cloud-core": "^1.49",
"google/gax": "^1.1"
},
"require-dev": {
Expand Down
15 changes: 11 additions & 4 deletions Spanner/src/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Google\ApiCore\ValidationException;
use Google\Cloud\Core\Exception\AbortedException;
use Google\Cloud\Core\Exception\NotFoundException;
use Google\Cloud\Core\Exception\ServiceException;
use Google\Cloud\Core\Iam\Iam;
use Google\Cloud\Core\Iterator\ItemIterator;
use Google\Cloud\Core\LongRunning\LongRunningConnectionInterface;
Expand All @@ -37,6 +38,7 @@
use Google\Cloud\Spanner\Transaction;
use Google\Cloud\Spanner\V1\SpannerClient as GapicSpannerClient;
use Google\Cloud\Spanner\V1\TypeCode;
use Google\Rpc\Code;

/**
* Represents a Cloud Spanner Database.
Expand Down Expand Up @@ -871,11 +873,16 @@ public function runTransaction(callable $operation, array $options = [])
};

$delayFn = function (\Exception $e) {
if (!($e instanceof AbortedException)) {
throw $e;
if ($e instanceof AbortedException) {
return $e->getRetryDelay();
}

return $e->getRetryDelay();
if ($e instanceof ServiceException &&
$e->getCode() === Code::INTERNAL &&
strpos($e->getMessage(), 'RST_STREAM') !== false
) {
return $e->getRetryDelay();
}
throw $e;
};

$transactionFn = function ($operation, $session, $options) use ($startTransactionFn) {
Expand Down
27 changes: 27 additions & 0 deletions Spanner/tests/Unit/DatabaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

use Google\Cloud\Core\Exception\AbortedException;
use Google\Cloud\Core\Exception\NotFoundException;
use Google\Cloud\Core\Exception\ServerException;
use Google\Cloud\Core\Iam\Iam;
use Google\Cloud\Core\Iterator\ItemIterator;
use Google\Cloud\Core\LongRunning\LongRunningConnectionInterface;
Expand Down Expand Up @@ -46,6 +47,7 @@
use Google\Cloud\Spanner\Timestamp;
use Google\Cloud\Spanner\Transaction;
use Google\Cloud\Spanner\V1\SpannerClient;
use Google\Rpc\Code;
use Yoast\PHPUnitPolyfills\TestCases\TestCase;
use Prophecy\Argument;
use Yoast\PHPUnitPolyfills\Polyfills\ExpectException;
Expand Down Expand Up @@ -745,6 +747,31 @@ public function testRunTransactionNestedTransaction()
});
}

public function testRunTransactionShouldRetryOnRstStreamErrors()
{
$this->expectException('Google\Cloud\Core\Exception\ServerException');
$this->expectExceptionMessage('RST_STREAM');
$err = new ServerException('RST_STREAM', Code::INTERNAL);

$this->connection->beginTransaction(Argument::allOf(
Argument::withEntry('session', $this->session->name()),
Argument::withEntry(
'database',
DatabaseAdminClient::databaseName(
self::PROJECT,
self::INSTANCE,
self::DATABASE
)
)
))
->shouldBeCalledTimes(3)
->willThrow($err);

$this->database->runTransaction(function ($t) {
// do nothing
}, ['maxRetries' => 2]);
}

public function testRunTransactionRetry()
{
$abort = new AbortedException('foo', 409, null, [
Expand Down