From 32bcff36e40949f1dff1cebccd12a062597cd3cd Mon Sep 17 00:00:00 2001 From: Oleksii Prudkyi Date: Sat, 14 May 2022 19:45:26 +0300 Subject: [PATCH] feat: handle 'session not found' errors --- README.md | 62 ++++++ phpstan.neon | 2 - .../Spanner/Concerns/ManagesSessionPool.php | 3 +- .../Spanner/Concerns/ManagesStaleReads.php | 10 +- .../Spanner/Concerns/ManagesTransactions.php | 54 +++-- src/Colopl/Spanner/Connection.php | 100 ++++++++- tests/Colopl/Spanner/SessionNotFoundTest.php | 207 ++++++++++++++++++ 7 files changed, 410 insertions(+), 28 deletions(-) create mode 100644 tests/Colopl/Spanner/SessionNotFoundTest.php diff --git a/README.md b/README.md index a69fe43a..82a987a0 100644 --- a/README.md +++ b/README.md @@ -236,6 +236,68 @@ In order to improve the performance of the first connection per request, we use By default, laravel-spanner uses [Filesystem Cache Adapter](https://symfony.com/doc/current/components/cache/adapters/filesystem_adapter.html) as the caching pool. If you want to use your own caching pool, you can extend ServiceProvider and inject it into the constructor of `Colopl\Spanner\Connection`. + +### 'Session not found' exception handling +There are a few cases when a 'Session not found' error can +[happen](https://cloud.google.com/spanner/docs/sessions#handle_deleted_sessions): + + - Scripts that idle too long - for example, a [Laravel queue worker](https://laravel.com/docs/9.x/queues#the-queue-work-command) +or anything that doesn't call Spanner frequently enough (more than once an hour). + - The session is more than 28 days old. + - Some random flukes on Google's side. + +The errors can be handled by one of the supported modes: + +- **MAINTAIN_SESSION_POOL** - When the 'session not found' error is encountered, the library tries to disconnect, +[maintain a session pool](https://github.com/googleapis/google-cloud-php/blob/077810260b58f5de8a3bbdfd999a5e9a48f71a7f/Spanner/src/Session/CacheSessionPool.php#L864) +(to remove outdated sessions), reconnect, and then try querying again. +```php + 'spanner' => [ + 'driver' => 'spanner', + ... + 'sessionNotFoundErrorMode' => 'MAINTAIN_SESSION_POOL', + ] +``` + +- **CLEAR_SESSION_POOL** (default) - The **MAINTAIN_SESSION_POOL** mode is tried first. If the error still happens, then +the [clearing of the session pool](https://github.com/googleapis/google-cloud-php/blob/077810260b58f5de8a3bbdfd999a5e9a48f71a7f/Spanner/src/Session/CacheSessionPool.php#L465) +is enforced and the query is tried once again. +As a consequence of session pool clearing, all processes that share the current session pool will be forced +to use the new session on the next call. The mode is enabled by default, but you can enable it explicitly via congifuration: +```php + 'spanner' => [ + 'driver' => 'spanner', + ... + 'sessionNotFoundErrorMode' => 'CLEAR_SESSION_POOL' + ] +``` + +- none - The QueryException is raised and the client code is free to handle it by itself.: +```php + 'spanner' => [ + 'driver' => 'spanner', + ... + 'sessionNotFoundErrorMode' => false, + ] +``` + +Please note, that [`getDatabaseContext()->execute(...)->rows()`](https://github.com/googleapis/google-cloud-php/blob/077810260b58f5de8a3bbdfd999a5e9a48f71a7f/Spanner/src/Result.php#L175) +returns a `/Generator` object, which only accesses Spanner when iterated. That affects `cursor()` +and `cursorWithTimestampBound()` functions and many low-level calls. So you might still +get `Google\Cloud\Core\Exception\NotFoundException` when trying to resolve cursor. +To avoid that, please run cursor* functions inside +[explicit transactions](#transactions) so statements will repeat on error. + +```php +$conn->transaction(function () use ($conn) { + $cursor = $conn->cursor('SELECT ...'); + + foearch ($cursor as $value) { ... +}); +``` + + + ### Laravel Tinker You can use [Laravel Tinker](https://github.com/laravel/tinker) with commands such as `php artisan tinker`. But your session may hang when accessing Cloud Spanner. This is known gRPC issue that occurs when PHP forks a process. diff --git a/phpstan.neon b/phpstan.neon index 54d8dcce..aa06e85f 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -16,8 +16,6 @@ parameters: path: src/Colopl/Spanner/Connection.php - message: '#^Method Colopl\\Spanner\\Connection::getSpannerDatabase\(\) should return Google\\Cloud\\Spanner\\Database but returns Google\\Cloud\\Spanner\\Database\|null.$#' path: src/Colopl/Spanner/Connection.php - - message: '#^Method Colopl\\Spanner\\Connection::transaction\(\) should return T but returns mixed\.$#' - path: src/Colopl/Spanner/Connection.php - message: '#^Cannot cast mixed to int\.$#' path: src/Colopl/Spanner/Eloquent/Model.php - message: '#^Method Colopl\\Spanner\\Query\\Builder::insertGetId\(\) should return int but return statement is missing\.$#' diff --git a/src/Colopl/Spanner/Concerns/ManagesSessionPool.php b/src/Colopl/Spanner/Concerns/ManagesSessionPool.php index 92d0794a..99446f93 100644 --- a/src/Colopl/Spanner/Concerns/ManagesSessionPool.php +++ b/src/Colopl/Spanner/Concerns/ManagesSessionPool.php @@ -81,7 +81,8 @@ public function listSessions(): Collection { $databaseName = $this->getSpannerDatabase()->name(); $response = (new ProtobufSpannerClient())->listSessions($databaseName); - return collect($response->iterateAllElements())->map(function (ProtobufSpannerSession $session) { + return collect($response->iterateAllElements())->map(function ($session) { + assert($session instanceof ProtobufSpannerSession); return new Session($session); }); } diff --git a/src/Colopl/Spanner/Concerns/ManagesStaleReads.php b/src/Colopl/Spanner/Concerns/ManagesStaleReads.php index 241f2502..4b8a1234 100644 --- a/src/Colopl/Spanner/Concerns/ManagesStaleReads.php +++ b/src/Colopl/Spanner/Concerns/ManagesStaleReads.php @@ -56,7 +56,9 @@ public function cursorWithTimestampBound($query, $bindings = [], TimestampBoundI */ public function selectWithTimestampBound($query, $bindings = [], TimestampBoundInterface $timestampBound = null): array { - return iterator_to_array($this->cursorWithTimestampBound($query, $bindings, $timestampBound)); + return $this->sessionNotFoundWrapper(function () use ($query, $bindings, $timestampBound) { + return iterator_to_array($this->cursorWithTimestampBound($query, $bindings, $timestampBound)); + }); } /** @@ -68,7 +70,11 @@ public function selectWithTimestampBound($query, $bindings = [], TimestampBoundI */ public function selectOneWithTimestampBound($query, $bindings = [], TimestampBoundInterface $timestampBound = null): ?array { - return $this->cursorWithTimestampBound($query, $bindings, $timestampBound)->current(); + $result = $this->sessionNotFoundWrapper(function () use ($query, $bindings, $timestampBound) { + return $this->cursorWithTimestampBound($query, $bindings, $timestampBound)->current(); + }); + assert(is_null($result) || is_array($result)); + return $result; } } diff --git a/src/Colopl/Spanner/Concerns/ManagesTransactions.php b/src/Colopl/Spanner/Concerns/ManagesTransactions.php index 644a572a..012f2183 100644 --- a/src/Colopl/Spanner/Concerns/ManagesTransactions.php +++ b/src/Colopl/Spanner/Concerns/ManagesTransactions.php @@ -20,6 +20,7 @@ use Closure; use Exception; use Google\Cloud\Core\Exception\AbortedException; +use Google\Cloud\Core\Exception\NotFoundException; use Google\Cloud\Spanner\Database; use Google\Cloud\Spanner\Transaction; use Throwable; @@ -44,29 +45,31 @@ trait ManagesTransactions */ public function transaction(Closure $callback, $attempts = Database::MAX_RETRIES) { - // Since Cloud Spanner does not support nested transactions, - // we use Laravel's transaction management for nested transactions only. - if ($this->transactions > 0) { - return parent::transaction($callback, $attempts); - } - - $return = $this->getSpannerDatabase()->runTransaction(function (Transaction $tx) use ($callback) { - try { - $this->currentTransaction = $tx; - $this->transactions++; - $this->fireConnectionEvent('beganTransaction'); - $result = $callback($this); - $this->performSpannerCommit(); - return $result; - } catch (Throwable $e) { - $this->rollBack(); - throw $e; + return $this->sessionNotFoundWrapper(function () use ($callback, $attempts) { + // Since Cloud Spanner does not support nested transactions, + // we use Laravel's transaction management for nested transactions only. + if ($this->transactions > 0) { + return parent::transaction($callback, $attempts); } - }, ['maxRetries' => $attempts - 1]); - - $this->fireConnectionEvent('committed'); - return $return; + $return = $this->getSpannerDatabase()->runTransaction(function (Transaction $tx) use ($callback) { + try { + $this->currentTransaction = $tx; + $this->transactions++; + $this->fireConnectionEvent('beganTransaction'); + $result = $callback($this); + $this->performSpannerCommit(); + return $result; + } catch (Throwable $e) { + $this->rollBack(); + throw $e; + } + }, ['maxRetries' => $attempts - 1]); + + $this->fireConnectionEvent('committed'); + + return $return; + }); } /** @@ -158,7 +161,14 @@ protected function performRollBack($toLevel) if ($this->currentTransaction !== null) { if ($this->currentTransaction->state() === Transaction::STATE_ACTIVE) { - $this->currentTransaction->rollBack(); + try { + $this->currentTransaction->rollBack(); + } catch (NotFoundException $e) { + // ignore session not found error + if (empty($this->getSessionNotFoundMode()) || !$this->causedBySessionNotFound($e)) { + throw $e; + } + } } $this->currentTransaction = null; } diff --git a/src/Colopl/Spanner/Connection.php b/src/Colopl/Spanner/Connection.php index 7b773383..92378e8f 100644 --- a/src/Colopl/Spanner/Connection.php +++ b/src/Colopl/Spanner/Connection.php @@ -30,6 +30,7 @@ use Generator; use Google\Cloud\Core\Exception\AbortedException; use Google\Cloud\Core\Exception\GoogleException; +use Google\Cloud\Core\Exception\NotFoundException; use Google\Cloud\Spanner\Database; use Google\Cloud\Spanner\Session\SessionPoolInterface; use Google\Cloud\Spanner\SpannerClient; @@ -37,6 +38,7 @@ use Illuminate\Contracts\Support\Arrayable; use Illuminate\Database\Connection as BaseConnection; use Illuminate\Database\QueryException; +use InvalidArgumentException; use Psr\Cache\CacheItemPoolInterface; use RuntimeException; use Throwable; @@ -82,6 +84,21 @@ class Connection extends BaseConnection */ protected $sessionPool; + /** + * Try to maintain session pool on 'session not found' error + */ + public const MAINTAIN_SESSION_POOL = 'MAINTAIN_SESSION_POOL'; + + /** + * Try to maintain and then clear session pool on 'session not found' error + */ + public const CLEAR_SESSION_POOL = 'CLEAR_SESSION_POOL'; + + /** + * Used to detect specific exception + */ + public const SESSION_NOT_FOUND_CONDITION = 'Session does not exist'; + /** * @param string $instanceId instance ID * @param string $databaseName @@ -425,7 +442,9 @@ protected function runQueryCallback($query, $bindings, Closure $callback) [$query, $bindings] = $this->parameterizer->parameterizeQuery($query, $bindings); try { - $result = $callback($query, $bindings); + $result = $this->sessionNotFoundWrapper(function () use ($query, $bindings, $callback) { + return $callback($query, $bindings); + }); } // AbortedExceptions are expected to be thrown upstream by the Google Client Library upstream, @@ -443,4 +462,83 @@ protected function runQueryCallback($query, $bindings, Closure $callback) return $result; } + + /** + * Returns current mode + * + * @return string + */ + protected function getSessionNotFoundMode() + { + return $this->config['sessionNotFoundErrorMode'] ?? self::CLEAR_SESSION_POOL; + } + + /** + * Handle "session not found" errors + * + * @template T + * @param Closure(): T $callback + * @return T + * @throws InvalidArgumentException|NotFoundException|AbortedException + */ + protected function sessionNotFoundWrapper(Closure $callback) + { + $handlerMode = $this->getSessionNotFoundMode(); + if (empty($handlerMode) || $this->sessionPool === null) { + // skip handlers + return $callback(); + } + + if (!in_array($handlerMode, [ + self::MAINTAIN_SESSION_POOL, + self::CLEAR_SESSION_POOL, + ]) + ) { + throw new InvalidArgumentException("Unsupported sessionNotFoundErrorMode [{$handlerMode}]."); + } + try { + return $callback(); + } catch (NotFoundException $e) { + // ensure if this really error with session + if ($this->causedBySessionNotFound($e)) { + if ($this->inTransaction()) { + // if we inside transaction then throw abort exception + throw new AbortedException(self::SESSION_NOT_FOUND_CONDITION, $e->getCode(), $e); + } + $this->disconnect(); + // clear expired sessions, manually deleted sessions still raise error + $this->maintainSessionPool(); + $this->reconnect(); + try { + return $callback(); + } catch (NotFoundException $e) { + if ($handlerMode == self::CLEAR_SESSION_POOL && $this->causedBySessionNotFound($e)) { + $this->disconnect(); + // forcefully clearing sessions, might affect parallel processes + // also cleared sessions are still accounted toward spanner limit - 10k sessions per node + $this->clearSessionPool(); + $this->reconnect(); + return $callback(); + } else { + throw $e; + } + } + } else { + throw $e; + } + } + } + + /** + * Check if this is "session not found" error + * + * @param Throwable $e + * @return boolean + */ + public function causedBySessionNotFound(Throwable $e): bool + { + return ($e instanceof NotFoundException) + && strpos($e->getMessage(), self::SESSION_NOT_FOUND_CONDITION) !== false; + } + } diff --git a/tests/Colopl/Spanner/SessionNotFoundTest.php b/tests/Colopl/Spanner/SessionNotFoundTest.php new file mode 100644 index 00000000..c659adb1 --- /dev/null +++ b/tests/Colopl/Spanner/SessionNotFoundTest.php @@ -0,0 +1,207 @@ +getSpannerDatabase()->__debugInfo()['session']->delete(); + } + + public function testSessionNotFoundHandledError() + { + $conn = $this->getDefaultConnection(); + + $conn->selectOne('SELECT 1'); + + $this->deleteSession($conn); + + $this->assertEquals(12345, $conn->selectOne('SELECT 12345')[0]); + } + + public function testInTransactionSessionNotFoundHandledError() + { + $conn = $this->getDefaultConnection(); + + $passes = 0; + + $conn->transaction(function () use ($conn, &$passes) { + + if ($passes == 0) { + $this->deleteSession($conn); + $passes++; + } + + $this->assertEquals(12345, $conn->selectOne('SELECT 12345')[0]); + + $passes++; + }); + $this->assertEquals(2, $passes, 'Transaction should be called twice'); + } + + public function testInTransactionCommitSessionNotFoundHandledError() + { + $conn = $this->getDefaultConnection(); + + $passes = 0; + + $conn->transaction(function () use ($conn, &$passes) { + + $this->assertEquals(12345, $conn->selectOne('SELECT 12345')[0]); + + if ($passes == 0) { + $this->deleteSession($conn); + } + $passes++; + }); + $this->assertEquals(2, $passes, 'Transaction should be called twice'); + } + + public function testNestedTransactionsSessionNotFoundHandledError() + { + $conn = $this->getDefaultConnection(); + + $passes = 0; + + $conn->transaction(function () use ($conn, &$passes) { + $conn->transaction(function () use ($conn, &$passes) { + $this->assertEquals(12345, $conn->selectOne('SELECT 12345')[0]); + + if ($passes == 0) { + $this->deleteSession($conn); + } + $passes++; + }); + }); + $this->assertEquals(2, $passes, 'Transaction should be called twice'); + } + + public function testCusrorSessionNotFoundHandledError() + { + $conn = $this->getDefaultConnection(); + + $passes = 0; + + $conn->transaction(function () use ($conn, &$passes) { + $cursor = $conn->cursor('SELECT 12345'); + + if ($passes == 0) { + $this->deleteSession($conn); + $passes++; + } + + $this->assertEquals(12345, $cursor->current()[0]); + + $passes++; + }); + $this->assertEquals(2, $passes, 'Transaction should be called twice'); + } + + public function testSessionNotFoundUnhandledError() + { + $config = $this->app['config']->get('database.connections.main'); + + // old behavior, just raise QueryException + $config['sessionNotFoundErrorMode'] = false; + + $cacheItemPool = new ArrayAdapter(); + $cacheSessionPool = new CacheSessionPool($cacheItemPool); + $conn = new Connection($config['instance'], $config['database'], '', $config, null, $cacheSessionPool); + $this->assertInstanceOf(Connection::class, $conn); + + $conn->selectOne('SELECT 1'); + $this->assertNotEmpty($cacheItemPool->getValues(), 'After executing some query, cache is created.'); + + // deliberately delete session on spanner side + $this->deleteSession($conn); + + $this->expectException(QueryException::class); + + // that string is used in sessionNotFoundWrapper() to catch session not found error, + // if google changes it then string should be changed in sessionNotFoundWrapper() as well + $this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION); + + $conn->selectOne('SELECT 1'); + } + + public function testCursorSessionNotFoundUnhandledError() + { + $config = $this->app['config']->get('database.connections.main'); + + // old behavior, just raise QueryException + $config['sessionNotFoundErrorMode'] = false; + + $cacheItemPool = new ArrayAdapter(); + $cacheSessionPool = new CacheSessionPool($cacheItemPool); + $conn = new Connection($config['instance'], $config['database'], '', $config, null, $cacheSessionPool); + $this->assertInstanceOf(Connection::class, $conn); + + $cursor = $conn->cursor('SELECT 1'); + $this->assertNotEmpty($cacheItemPool->getValues(), 'After executing some query, cache is created.'); + + // deliberately delete session on spanner side + $this->deleteSession($conn); + + $this->expectException(NotFoundException::class); + + // that string is used in sessionNotFoundWrapper() to catch session not found error, + // if google changes it then string should be changed in sessionNotFoundWrapper() as well + $this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION); + + iterator_to_array($cursor); + } + + public function testInTransactionSessionNotFoundUnhandledError() + { + $config = $this->app['config']->get('database.connections.main'); + + // old behavior, just raise QueryException + $config['sessionNotFoundErrorMode'] = false; + + $cacheItemPool = new ArrayAdapter(); + $cacheSessionPool = new CacheSessionPool($cacheItemPool); + $conn = new Connection($config['instance'], $config['database'], '', $config, null, $cacheSessionPool); + $this->assertInstanceOf(Connection::class, $conn); + + $this->expectException(NotFoundException::class); + + // that string is used in sessionNotFoundWrapper() to catch session not found error, + // if google changes it then string should be changed in sessionNotFoundWrapper() as well + $this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION); + + $passes = 0; + + $conn->transaction(function () use ($conn, &$passes) { + + if ($passes == 0) { + $this->deleteSession($conn); + $passes++; + } + + $conn->selectOne('SELECT 12345'); + }); + } +}