Skip to content

Commit 920e1bc

Browse files
authored
fix(Spanner): prevent unnecessary GCE metadata probing during client … (#9327)
* fix(Spanner): prevent unnecessary GCE metadata probing during client initialization * test(spanner): use insertOrUpdate to prevent ConflictException Changes insert to insertOrUpdate and insertBatch to insertOrUpdateBatch in Transaction and PartitionedDML system tests. This prevents ConflictException (ALREADY_EXISTS) when tests are retried or a transaction closure is retried after a transient emulator error (e.g. ABORTED) where the initial mutation was still applied.
1 parent 2e12179 commit 920e1bc

6 files changed

Lines changed: 33 additions & 11 deletions

File tree

Spanner/src/SpannerClient.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1057,7 +1057,6 @@ private function configureMetrics(array $options): void
10571057
{
10581058
$metricsClient = $this->pluck('metricServiceClient', $options, false);
10591059
$timeoutMillis = $this->pluck('metricsTimeoutMillis', $options, false) ?? 100;
1060-
$location = $this->getLocation();
10611060

10621061
if (!$this->pluck('enableBuiltInMetrics', $options, false)) {
10631062
return;
@@ -1087,6 +1086,7 @@ private function configureMetrics(array $options): void
10871086
throw new ValidationException('The "metricServiceClient" option must be a MetricServiceClient instance.');
10881087
}
10891088

1089+
$location = $this->getLocation();
10901090
$metricsClientId = RUUID::uuid4()->toString() . '-' . getmypid();
10911091
$exporter = new MetricsExporter($metricsClient, $this->projectId, $metricsClientId, $timeoutMillis);
10921092
$reader = new ExportingReader($exporter);

Spanner/tests/System/PartitionedDmlTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ private function seedTable()
8888
private function executeInsert(array $rows)
8989
{
9090
self::$database->runTransaction(function ($t) use ($rows) {
91-
$t->insertBatch(self::PDML_TABLE, $rows);
91+
$t->insertOrUpdateBatch(self::PDML_TABLE, $rows);
9292

9393
$t->commit();
9494
});

Spanner/tests/System/PgPartitionedDmlTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public function testPdml()
4747

4848
$db = self::$database;
4949

50-
$db->updateDdl('CREATE TABLE ' . self::PDML_TABLE . '(
50+
$db->updateDdl('CREATE TABLE IF NOT EXISTS ' . self::PDML_TABLE . '(
5151
id bigint NOT NULL,
5252
stringField varchar(1024),
5353
boolField BOOL,
@@ -95,7 +95,7 @@ private function seedTable()
9595
private function executeInsert(array $rows)
9696
{
9797
self::$database->runTransaction(function ($t) use ($rows) {
98-
$t->insertBatch(self::PDML_TABLE, $rows);
98+
$t->insertOrUpdateBatch(self::PDML_TABLE, $rows);
9999

100100
$t->commit();
101101
});

Spanner/tests/System/PgTransactionTest.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public static function setUpTestFixtures(): void
5353
self::$tableName = 'transactions_test';
5454

5555
self::$database->updateDdlBatch([
56-
'CREATE TABLE ' . self::$tableName . ' (
56+
'CREATE TABLE IF NOT EXISTS ' . self::$tableName . ' (
5757
id bigint NOT NULL,
5858
number bigint NOT NULL,
5959
PRIMARY KEY (id)
@@ -77,7 +77,7 @@ public function testRunTransaction()
7777

7878
$db->runTransaction(function ($t) {
7979
$id = rand(1, 346464);
80-
$t->insert(self::TEST_TABLE_NAME, [
80+
$t->insertOrUpdate(self::TEST_TABLE_NAME, [
8181
'id' => $id,
8282
'name' => uniqid(self::TESTING_PREFIX),
8383
'birthday' => new Date(new \DateTime('2000-01-01'))
@@ -142,15 +142,16 @@ public function testRunTransactionWithDbRole($db, $values, $expected)
142142
$this->skipEmulatorTests();
143143

144144
$error = null;
145+
$newName = uniqid('Doug');
145146
$row = $this->getRow();
146-
$row['name'] = 'Doug';
147+
$row['name'] = $newName;
147148

148149
$db->runTransaction(function ($t) use ($row) {
149150
$t->update(self::TEST_TABLE_NAME, $row);
150151
$t->commit();
151152
});
152153
$row = $this->getRow();
153-
$this->assertEquals('Doug', $row['name']);
154+
$this->assertEquals($newName, $row['name']);
154155

155156
try {
156157
$db->runTransaction(function ($t) use ($values) {

Spanner/tests/System/TransactionTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public function testRunTransaction()
9393
$cols = array_keys($row);
9494

9595
$db->runTransaction(function ($t) use ($row) {
96-
$t->insert(self::TEST_TABLE_NAME, $row);
96+
$t->insertOrUpdate(self::TEST_TABLE_NAME, $row);
9797
$t->commit();
9898
});
9999

@@ -272,15 +272,16 @@ public function testRunTransactionWithDbRole($db, $values, $expected)
272272
$this->skipEmulatorTests();
273273

274274
$error = null;
275+
$newName = uniqid('Doug');
275276
$row = $this->getRow();
276-
$row['name'] = 'Doug';
277+
$row['name'] = $newName;
277278

278279
$db->runTransaction(function ($t) use ($row) {
279280
$t->update(self::TEST_TABLE_NAME, $row);
280281
$t->commit();
281282
});
282283
$row = $this->getRow();
283-
$this->assertEquals('Doug', $row['name']);
284+
$this->assertEquals($newName, $row['name']);
284285

285286
try {
286287
$db->runTransaction(function ($t) use ($values) {

Spanner/tests/Unit/SpannerClientTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,4 +900,24 @@ public function testBuiltinMetricsCanBeEnabled()
900900
'enableBuiltInMetrics' => true,
901901
]);
902902
}
903+
904+
/**
905+
* @runInSeparateProcess
906+
*/
907+
public function testSpannerClientInstantiatesWithoutDelayWhenMetricsDisabled()
908+
{
909+
// This test ensures that the SpannerClient does not hit the GCE metadata server
910+
// when enableBuiltInMetrics is false, which prevents a 1.5 second delay.
911+
// It's run in a separate process to ensure the GCE static cache is empty.
912+
$start = microtime(true);
913+
new SpannerClient([
914+
'projectId' => self::PROJECT,
915+
'credentials' => Fixtures::KEYFILE_STUB_FIXTURE()
916+
]);
917+
$end = microtime(true);
918+
919+
// Assert that the client instantiated quickly.
920+
// If it probed the GCE metadata server without a mock, it would take ~1.5s to time out.
921+
$this->assertLessThan(1.0, $end - $start);
922+
}
903923
}

0 commit comments

Comments
 (0)