feat!: Remove handwritten Retry logic

Spanner/src/Database.php

@@ -17,6 +17,7 @@
 
 namespace Google\Cloud\Spanner;
 
+use Google\ApiCore\RetrySettings;
 use Google\ApiCore\Serializer;
 use Google\ApiCore\ValidationException;
 use Google\Cloud\Core\ApiHelperTrait;
@@ -453,6 +454,10 @@ public function exists(array $options = [])
      *     Configuration Options
      *
      *     @type string[] $statements Additional DDL statements.
+     *     @type RetrySettings|array $retrySettings
+     *           Retry settings to use for this call. Can be a {@see RetrySettings} object, or an
+     *           associative array of retry settings parameters. See the documentation on
+     *           {@see RetrySettings} for example usage.
      * }
      * @return LongRunningOperationManager<Database>
      */
@@ -969,6 +974,7 @@ public function runTransaction(callable $operation, array $options = [])
             $this->pluck('sessionOptions', $options, false) ?: []
         );
 
+        # TODO: Discuss the fact that we can not remove this ILB logic.
         $attempt = 0;
         $startTransactionFn = function ($session, $options) use (&$attempt) {
 
@@ -1034,6 +1040,78 @@ public function runTransaction(callable $operation, array $options = [])
         }
     }
 
+    // An example to show runTransaction function's retry is different than that of the APIs.
+    public function runTransaction1(callable $operation, array $options = [])
+    {
+        if ($this->isRunningTransaction) {
+            throw new \BadMethodCallException('Nested transactions are not supported by this client.');
+        }
+
+        $options += [
+            'maxRetries' => self::MAX_RETRIES,
+        ];
+
+        // There isn't anything configurable here.
+        $options['transactionOptions'] = $this->configureTransactionOptions();
+
+        $session = $this->selectSession(
+            SessionPoolInterface::CONTEXT_READWRITE,
+            $this->pluck('sessionOptions', $options, false) ?: []
+        );
+        if (!isset($options['transactionOptions']['partitionedDml'])) {
+            $options['begin'] = $options['transactionOptions'];
+        }
+        $this->isRunningTransaction = true;
+
+        $delayFn = function (\Exception $e) {
+            if ($e instanceof AbortedException) {
+                return $e->getRetryDelay();
+            }
+            if ($e instanceof ServiceException &&
+                $e->getCode() === Code::INTERNAL &&
+                strpos($e->getMessage(), 'RST_STREAM') !== false
+            ) {
+                return $e->getRetryDelay();
+            }
+            throw $e;
+        };
+        $transaction = $this->operation->transaction($session, $options);
+        try {
+            // This is a block created by the user, there is no single API to retry here.
+            // [PROBLEM] Hence, using API level retry will NOT be a fit.
+            $res = call_user_func($operation, $transaction);
+        } catch (\Exception $e) {
+            // We can safely retry as this is our first attempt.
+            $delay = $delayFn($e) + [
+                'seconds' => 0,
+                'nanos' => 0
+            ];
+            time_nanosleep($delay['seconds'], $delay['nanos']);
+            $options['isRetry'] = true;
+            $transaction = $this->operation->transaction($session, $options);
+            // This is a code block created by the user, there is no single API to retry here.
+            // [PROBLEM] Hence, using API level retry will NOT be a fit.
+            try {
+                $res = call_user_func($operation, $transaction);
+            } catch (\Exception $e) {
+                $transaction->rollback($options);
+                throw $e;
+            }
+        } finally {
+            $this->isRunningTransaction = false;
+            $session->setExpiration();
+        }
+
+        $active = $transaction->state() === Transaction::STATE_ACTIVE;
+        $singleUse = $transaction->type() === Transaction::TYPE_SINGLE_USE;
+        if ($active && !$singleUse) {
+            $transaction->rollback($options);
+            throw new \RuntimeException('Transactions must be rolled back or committed.');
+        }
+
+        return $res;
+    }
+
     /**
      * Insert a row.
      *

Spanner/src/Operation.php

@@ -245,6 +245,7 @@ public function execute(Session $session, $sql, array $options = [])
         // Initially with begin, transactionId will be null.
         // Once transaction is generated, even in the case of stream failure,
         // transaction will be passed to this callable by the Result class.
+        // TODO: Discuss executestreamingsql and streamingRead does NOT have retrySettings
         $call = function ($resumeToken = null, $transaction = null) use (
             $session,
             $sql,
@@ -447,6 +448,7 @@ public function read(
 
         $context = $this->pluck('transactionContext', $options);
 
+        // TODO: Discuss executestreamingsql and streamingRead does NOT have retrySettings
         $call = function ($resumeToken = null, $transaction = null) use (
             $table,
             $session,

Spanner/src/Result.php

@@ -227,13 +227,14 @@ public function rows($format = self::RETURN_ASSOCIATIVE)
                 $valid = $this->generator->valid();
             } catch (ServiceException $ex) {
                 if ($shouldRetry && $ex->getCode() === Grpc\STATUS_UNAVAILABLE) {
+                    // TODO: Discuss executestreamingsql and streamingRead does NOT have retrySettings
                     $backoff = new ExponentialBackoff($this->retries, function ($ex) {
                         return $ex instanceof ServiceException &&
                             $ex->getCode() === Grpc\STATUS_UNAVAILABLE;
                     });
                     // Attempt to resume using the last stored resume token and the transaction.
                     // If we successfully resume, flush the buffer.
-                    $this->generator = $backoff->execute($call, [$this->resumeToken, $this->transaction()]);
+                    $this->generator = $backoff->execute($call, [$this->resumeToken, $this->transaction]);
                     $bufferedResults = [];
 
                     continue;
@@ -525,6 +526,7 @@ private function createGenerator()
         $call = $this->call;
         $generator = null;
 
+        // TODO: Discuss executestreamingsql and streamingRead does NOT have retrySettings
         $backoff = new ExponentialBackoff($this->retries, function ($ex) {
             return $ex instanceof ServiceException &&
                 $ex->getCode() === Grpc\STATUS_UNAVAILABLE;

Spanner/src/SpannerClient.php

@@ -199,6 +199,15 @@ class SpannerClient
      *           request. **Defaults to** `0` with REST and `60` with gRPC.
      *     @type int $retries Number of retries for a failed request. Used only
      *           with default backoff strategy. **Defaults to** `3`.
+     *           TODO: Discuss the need of removing this option. This used to go to the GrpcRequestWrapper and used
+     *           here: https://github.com/googleapis/google-cloud-php/blob/main/Core/src/RequestWrapperTrait.php#L124.
+     *           I dont think it get used in the requestHandler at constructor level, because these options directly been
+     *           send to the V1/Client classes and they expects retry options in the constructor under ClientOptions.
+     *           However, I don't see the use of this maxRetries anywhere. See any V1/Client constructors for more details.
+     *           Furthermore, I see that some APIs does not have the retry capability, eg: SpannerClient.streamingRead and
+     *           settting this at constructor level of requestHandler may not be the correct way. 2 Options:
+     *           1. SpannerClient pass this config to all the APIs internally
+     *           2. Remove maxRetries from here and enable it at API level (eg: see lateset Database.create implementaiton).
      *     @type array $scopes Scopes to be used for the request.
      *     @type string $quotaProject Specifies a user project to bill for
      *           access charges associated with the request.
@@ -220,10 +229,6 @@ class SpannerClient
      *           query execution. Executing a SQL statement with an invalid
      *           optimizer version will fail with a syntax error
      *           (`INVALID_ARGUMENT`) status.
-     *     @type bool $useDiscreteBackoffs `false`: use default backoff strategy
-     *           (retry every failed request up to `retries` times).
-     *           `true`: use discrete backoff settings based on called method name.
-     *           **Defaults to** `false`.
      *     @type array $directedReadOptions Directed read options.
      *           {@see \Google\Cloud\Spanner\V1\DirectedReadOptions}
      *           If using the `replicaSelection::type` setting, utilize the constants available in
@@ -247,25 +252,9 @@ public function __construct(array $config = [])
             'projectIdRequired' => true,
             'hasEmulator' => (bool) $emulatorHost,
             'emulatorHost' => $emulatorHost,
-            'queryOptions' => [],
-            'transportConfig' => [
-                'grpc' => [
-                    // increase default limit to 4MB to prevent metadata exhausted errors
-                    'stubOpts' => ['grpc.max_metadata_size' => 4 * 1024 * 1024,]
-                ]
-            ]
+            'queryOptions' => []
         ];
 
-        # Check this to see it needs to be removed, for now keep it.
-        if (!empty($config['useDiscreteBackoffs'])) {
-            $config = array_merge_recursive($config, [
-                'retries' => 0,
-                'grpcOptions' => [
-                    'retrySettings' => [],
-                ],
-            ]);
-        }
-
         # TODO: Remove the connection related objects
         $this->connection = new Grpc($this->configureAuthentication($config));
         $this->returnInt64AsObject = $config['returnInt64AsObject'];

Spanner/src/Transaction.php

@@ -86,7 +86,8 @@ class Transaction implements TransactionalReadInterface
      * @param Session $session The session to use for spanner interactions.
      * @param string $transactionId [optional] The Transaction ID. If no ID is
      *        provided, the Transaction will be a Single-Use Transaction.
-     * @param bool $isRetry Whether the transaction will automatically retry or not.
+     * @param bool $isRetry Indicates whether the current transaction is
+     *         the result of a retry operation.
      * @param string $tag A transaction tag. Requests made using this transaction will
      *        use this as the transaction tag.
      * @param array $options [optional] {