chore: make precommit token check emulator-proof

The Emulator returns an empty pre-commit token when a commit is attempted
without a pre-commit token. This is different from not returning any
pre-commit token at all. The check for 'did the Commit return a pre-commit
token?' did not take this into account, which caused commits on the
Emulator that needed to be retried, not to be retried. This again caused
multiple test errors when running on the Emulator, as this would keep a
transaction present on the test database on the Emulator, and the Emulator
only supports one transaction at a time. These test failures went unnoticed,
because the test configuration for the Emulator had pinned the Emulator
version to 1.5.37, which did not support multiplexed sessions. This again
caused the tests to fall back to using regular sessions.

This change fixes the check for whether a pre-commit token was returned by
a Commit. It also unpins the Emulator version for the system tests using
default settings. This ensures that the tests actually use multiplexed
sessions.

Author: olavloite

.github/workflows/integration-tests-against-emulator.yaml

@@ -10,7 +10,7 @@ jobs:
 
     services:
       emulator:
-        image: gcr.io/cloud-spanner-emulator/emulator:1.5.37
+        image: gcr.io/cloud-spanner-emulator/emulator
         ports:
           - 9010:9010
           - 9020:9020

google/cloud/spanner_v1/snapshot.py

@@ -133,7 +133,7 @@ def _restart_on_unavailable(
                 # Update the transaction from the response.
                 if transaction is not None:
                     transaction._update_for_result_set_pb(item)
-                if item.precommit_token is not None and transaction is not None:
+                if item._pb.HasField("precommit_token") and transaction is not None:
                     transaction._update_for_precommit_token_pb(item.precommit_token)
 
                 if item.resume_token:
@@ -1029,7 +1029,7 @@ def _update_for_transaction_pb(self, transaction_pb: Transaction) -> None:
         if self._transaction_id is None and transaction_pb.id:
             self._transaction_id = transaction_pb.id
 
-        if transaction_pb.precommit_token:
+        if transaction_pb._pb.HasField("precommit_token"):
             self._update_for_precommit_token_pb_unsafe(transaction_pb.precommit_token)
 
     def _update_for_precommit_token_pb(

google/cloud/spanner_v1/transaction.py

@@ -328,14 +328,20 @@ def before_next_retry(nth_retry, delay_in_seconds):
             # successfully commit, and must be retried with the new precommit token.
             # The mutations should not be included in the new request, and no further
             # retries or exception handling should be performed.
-            if commit_response_pb.precommit_token:
+            if commit_response_pb._pb.HasField("precommit_token"):
                 add_span_event(span, commit_retry_event_name)
+                nth_request = database._next_nth_request
                 commit_response_pb = api.commit(
                     request=CommitRequest(
                         precommit_token=commit_response_pb.precommit_token,
                         **common_commit_request_args,
                     ),
-                    metadata=metadata,
+                    metadata=database.metadata_with_request_id(
+                        nth_request,
+                        1,
+                        metadata,
+                        span,
+                    ),
                 )
 
             add_span_event(span, "Commit Done")
@@ -521,7 +527,7 @@ def wrapped_method(*args, **kwargs):
         if is_inline_begin:
             self._lock.release()
 
-        if result_set_pb.precommit_token is not None:
+        if result_set_pb._pb.HasField("precommit_token"):
             self._update_for_precommit_token_pb(result_set_pb.precommit_token)
 
         return result_set_pb.stats.row_count_exact

tests/unit/test_transaction.py

@@ -533,7 +533,7 @@ def _commit_helper(
             )
             commit.assert_any_call(
                 request=expected_retry_request,
-                metadata=base_metadata,
+                metadata=expected_retry_metadata,
             )
 
         if not HAS_OPENTELEMETRY_INSTALLED: