Skip to content

Commit 1af3535

Browse files
authored
test(spanner): retry internal errors from the Emulator (#5840)
The latest version of the Spanner Emulator can return an INTERNAL error if DDL statements and read/write transactions are executed concurrently. This change adds a specific error to the set of errors that are retried by the Spanner client when it is running agains the Emulator. This additional check is only done if the client is connected to the Emulator, and not if the client is connected to real Spanner. Note: The test that failed and triggered the creation of this ticket, is not the actual culprit, and a similar failure could in theory happen for any of the other tests for the Spanner client that execute a read/write transaction. Fixes #5826
1 parent c7f029a commit 1af3535

7 files changed

Lines changed: 210 additions & 85 deletions

src/spanner/src/client.rs

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ use crate::model::{
2020
};
2121
use crate::server_streaming::builder;
2222
use gaxi::options::{ClientConfig, Credentials};
23+
use google_cloud_auth::credentials::anonymous;
2324
use google_cloud_gax::client_builder::ClientBuilder as GaxClientBuilder;
25+
use google_cloud_gax::client_builder::internal::new_builder;
2426
use google_cloud_gax::options::{
2527
RequestOptions as GaxRequestOptions, internal::RequestOptionsExt as _,
2628
};
@@ -49,6 +51,7 @@ pub struct Spanner {
4951
pub(crate) channels: Vec<Channel>,
5052
pub(crate) counter: std::sync::Arc<AtomicUsize>,
5153
pub(crate) config: ClientConfig,
54+
pub(crate) is_emulator: bool,
5255
}
5356

5457
/// A factory for constructing `Spanner` clients.
@@ -58,7 +61,21 @@ impl google_cloud_gax::client_builder::internal::ClientFactory for Factory {
5861
type Client = Spanner;
5962
type Credentials = Credentials;
6063

61-
async fn build(self, config: ClientConfig) -> crate::ClientBuilderResult<Self::Client> {
64+
async fn build(self, mut config: ClientConfig) -> crate::ClientBuilderResult<Self::Client> {
65+
let mut is_emulator = false;
66+
if let Some(endpoint) = std::env::var("SPANNER_EMULATOR_HOST")
67+
.ok()
68+
.filter(|s| !s.is_empty())
69+
{
70+
is_emulator = true;
71+
if config.endpoint.is_none() {
72+
config.endpoint = Some(parse_emulator_endpoint(&endpoint));
73+
}
74+
if config.cred.is_none() {
75+
config.cred = Some(anonymous::Builder::new().build());
76+
}
77+
}
78+
6279
let num_channels = std::env::var("SPANNER_NUM_CHANNELS")
6380
.ok()
6481
.and_then(|s| s.parse::<usize>().ok())
@@ -73,6 +90,7 @@ impl google_cloud_gax::client_builder::internal::ClientFactory for Factory {
7390
channels,
7491
counter: std::sync::Arc::new(AtomicUsize::new(0)),
7592
config,
93+
is_emulator,
7694
})
7795
}
7896
}
@@ -175,22 +193,7 @@ impl Spanner {
175193
/// detects and connects to the Spanner emulator if the `SPANNER_EMULATOR_HOST`
176194
/// environment variable is set.
177195
pub fn builder() -> ClientBuilder {
178-
let builder = google_cloud_gax::client_builder::internal::new_builder(Factory);
179-
// The Spanner client should automatically use the Spanner emulator if the
180-
// SPANNER_EMULATOR_HOST environment variable is set.
181-
let Some(endpoint) = std::env::var("SPANNER_EMULATOR_HOST")
182-
.ok()
183-
.filter(|s| !s.is_empty())
184-
else {
185-
return builder;
186-
};
187-
188-
// Determine if we need to prefix the endpoint with a scheme
189-
let full_endpoint = parse_emulator_endpoint(&endpoint);
190-
191-
builder
192-
.with_endpoint(full_endpoint)
193-
.with_credentials(google_cloud_auth::credentials::anonymous::Builder::new().build())
196+
new_builder(Factory)
194197
}
195198

196199
/// Returns a builder for the [DatabaseAdmin] client.
@@ -221,11 +224,7 @@ impl Spanner {
221224
C: Clone + From<Credentials>,
222225
{
223226
if let Some(ref endpoint) = self.config.endpoint {
224-
let is_emulator = std::env::var("SPANNER_EMULATOR_HOST")
225-
.ok()
226-
.filter(|s| !s.is_empty())
227-
.is_some();
228-
let ep = map_emulator_admin_endpoint(endpoint, is_emulator);
227+
let ep = map_emulator_admin_endpoint(endpoint, self.is_emulator);
229228
builder = builder.with_endpoint(ep);
230229
}
231230
if let Some(ref cred) = self.config.cred {
@@ -279,9 +278,14 @@ impl Spanner {
279278
}],
280279
counter: std::sync::Arc::new(AtomicUsize::new(0)),
281280
config: ClientConfig::default(),
281+
is_emulator: false,
282282
}
283283
}
284284

285+
pub(crate) fn is_emulator(&self) -> bool {
286+
self.is_emulator
287+
}
288+
285289
pub(crate) fn get_channel(&self, hint: usize) -> &Channel {
286290
let idx = hint % self.channels.len();
287291
&self.channels[idx]

src/spanner/src/database_client.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ pub struct DatabaseClient {
5454
}
5555

5656
impl DatabaseClient {
57+
pub(crate) fn is_emulator(&self) -> bool {
58+
self.spanner.is_emulator()
59+
}
60+
5761
/// Returns a builder for a single-use read-only transaction.
5862
///
5963
/// # Example

src/spanner/src/partitioned_dml_transaction.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,9 @@ impl PartitionedDmlTransaction {
180180
let base_request = statement.into_request();
181181
let channel_hint = self.client.spanner.next_channel_hint();
182182
let client = self.client;
183+
let is_emulator = client.is_emulator();
183184

184-
// Execute the statement and retry if the transaction is aborted by Spanner.
185-
retry_aborted(&*self.retry_policy, || {
185+
let action = || {
186186
let begin_request = begin_request.clone();
187187
let base_request = base_request.clone();
188188
let session_name = session_name.clone();
@@ -214,8 +214,9 @@ impl PartitionedDmlTransaction {
214214

215215
extract_lower_bound_update_count_from_stream(stream).await
216216
}
217-
})
218-
.await
217+
};
218+
219+
retry_aborted(&*self.retry_policy, action, is_emulator).await
219220
}
220221

221222
fn amend_gax_options(&self, options: &mut GaxRequestOptions) {

src/spanner/src/read_write_transaction.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ use wkt::Duration;
6363
/// A builder for [ReadWriteTransaction].
6464
#[derive(Clone, Debug)]
6565
pub(crate) struct ReadWriteTransactionBuilder {
66-
client: DatabaseClient,
66+
pub(crate) client: DatabaseClient,
6767
options: TransactionOptions,
6868
transaction_tag: Option<String>,
6969
max_commit_delay: Option<Duration>,

0 commit comments

Comments
 (0)