diff --git a/.kokoro/presubmit/windows/test.bat b/.kokoro/presubmit/windows/test.bat index 3958de827d87..801c87f16cfb 100755 --- a/.kokoro/presubmit/windows/test.bat +++ b/.kokoro/presubmit/windows/test.bat @@ -6,7 +6,12 @@ CD github/google-cloud-php MKDIR %SHORT_JOB_NAME%\unit CALL php C:\Users\kbuilder\bin\composer self-update CALL php C:\Users\kbuilder\bin\composer update -CALL vendor/bin/phpunit --log-junit %SHORT_JOB_NAME%\unit\sponge_log.xml + +IF "%SHORT_JOB_NAME%" neq "php70" ( + CALL vendor/bin/phpunit --log-junit %SHORT_JOB_NAME%\unit\sponge_log.xml +) ELSE ( + CALL vendor/bin/phpunit +) if %errorlevel% neq 0 exit /b %errorlevel% RENAME C:\Users\kbuilder\software\php %SHORT_JOB_NAME% diff --git a/Spanner/src/Connection/ConnectionInterface.php b/Spanner/src/Connection/ConnectionInterface.php index 94908695bc01..b1b150beef21 100644 --- a/Spanner/src/Connection/ConnectionInterface.php +++ b/Spanner/src/Connection/ConnectionInterface.php @@ -130,6 +130,11 @@ public function createSession(array $args); */ public function createSessionAsync(array $args); + /** + * @param array $args + */ + public function batchCreateSessions(array $args); + /** * @param array $args */ diff --git a/Spanner/src/Connection/Grpc.php b/Spanner/src/Connection/Grpc.php index ed7761742822..b0a4f310e4a1 100644 --- a/Spanner/src/Connection/Grpc.php +++ b/Spanner/src/Connection/Grpc.php @@ -492,6 +492,24 @@ public function createSessionAsync(array $args) ); } + /** + * @param array $args + */ + public function batchCreateSessions(array $args) + { + $args['sessionTemplate'] = $this->serializer->decodeMessage( + new Session, + $this->pluck('sessionTemplate', $args) + ); + + $database = $this->pluck('database', $args); + return $this->send([$this->spannerClient, 'batchCreateSessions'], [ + $database, + $this->pluck('sessionCount', $args), + $this->addResourcePrefixHeader($args, $database) + ]); + } + /** * @param array $args */ diff --git a/Spanner/src/Session/CacheSessionPool.php b/Spanner/src/Session/CacheSessionPool.php index 5c9451010e59..bb1355fd7104 100644 --- a/Spanner/src/Session/CacheSessionPool.php +++ b/Spanner/src/Session/CacheSessionPool.php @@ -247,7 +247,7 @@ public function acquire($context = SessionPoolInterface::CONTEXT_READ) // Create a session if needed. if ($toCreate) { - $createdSessions = $this->createSessions(count($toCreate)); + $createdSessions = $this->createSessions(count($toCreate))[0]; $hasCreatedSessions = count($createdSessions) > 0; $session = $this->config['lock']->synchronize(function () use ( @@ -429,13 +429,8 @@ public function warmup() return 0; } - $createdSessions = []; $exception = null; - - try { - $createdSessions = $this->createSessions(count($toCreate)); - } catch (\Exception $exception) { - } + list ($createdSessions, $exception) = $this->createSessions(count($toCreate)); $this->config['lock']->synchronize(function () use ($toCreate, $createdSessions) { $item = $this->cacheItemPool->getItem($this->cacheKey); @@ -639,38 +634,40 @@ private function getSession(array &$data) * Creates sessions up to the count provided. * * @param int $count - * @return array + * @return [ array[] $sessions, \Exception $ex = null ] */ private function createSessions($count) { - $args = [ - 'database' => $this->database->name(), - 'session' => [ - 'labels' => isset($this->config['labels']) ? $this->config['labels'] : [] - ] - ]; - - $promises = []; - - for ($i = 0; $i < $count; $i++) { - $promises[] = $this->database->connection()->createSessionAsync($args); - } - - $results = Promise\settle($promises)->wait(); - $sessions = []; + $created = 0; + $exception = null; + + // Loop over RPC in case it returns less than the desired number of sessions. + // @see https://github.com/googleapis/google-cloud-php/pull/2342#discussion_r327925546 + while ($count > $created) { + try { + $res = $this->database->connection()->batchCreateSessions([ + 'database' => $this->database->name(), + 'sessionTemplate' => [ + 'labels' => isset($this->config['labels']) ? $this->config['labels'] : [] + ], + 'sessionCount' => $count - $created + ]); + } catch (\Exception $exception) { + break; + } - foreach ($results as $result) { - if ($result['state'] === 'fulfilled') { - $name = $result['value']->getName(); + foreach ($res['session'] as $result) { $sessions[] = [ - 'name' => $name, + 'name' => $result['name'], 'expiration' => $this->time() + SessionPoolInterface::SESSION_EXPIRATION_SECONDS ]; + + $created++; } } - return $sessions; + return [$sessions, $exception]; } /** diff --git a/Spanner/tests/System/SessionTest.php b/Spanner/tests/System/SessionTest.php new file mode 100644 index 000000000000..28ea2ab87011 --- /dev/null +++ b/Spanner/tests/System/SessionTest.php @@ -0,0 +1,102 @@ +identity(); + $cacheKey = sprintf( + CacheSessionPool::CACHE_KEY_TEMPLATE, + $identity['projectId'], + $identity['instance'], + $identity['database'] + ); + + $cache = new MemoryCacheItemPool; + $pool = new CacheSessionPool($cache, [ + 'maxSessions' => 10, + 'minSessions' => 5, + 'shouldWaitForSession' => false + ]); + $pool->setDatabase(self::$database); + + $this->assertNull($cache->getItem($cacheKey)->get()); + + $pool->warmup(); + + $this->assertPoolCounts($cache, $cacheKey, 5, 0, 0); + + $session = $pool->acquire(); + $this->assertInstanceOf(Session::class, $session); + $this->assertTrue($session->exists()); + $this->assertPoolCounts($cache, $cacheKey, 4, 1, 0); + $this->assertEquals($session->name(), current($cache->getItem($cacheKey)->get()['inUse'])['name']); + + $pool->release($session); + + $inUse = []; + for ($i = 0; $i < 10; $i++) { + $inUse[] = $pool->acquire(); + } + + $this->assertPoolCounts($cache, $cacheKey, 0, 10, 0); + + $exception = null; + try { + $pool->acquire(); + } catch (\RuntimeException $exception) { + // no-op + } + $this->assertInstanceOf( + \RuntimeException::class, + $exception, + 'Should catch a RuntimeException when pool is exhausted.' + ); + + foreach ($inUse as $i) { + $pool->release($i); + } + sleep(1); + + $this->assertPoolCounts($cache, $cacheKey, 10, 0, 0); + + $pool->clear(); + sleep(1); + $this->assertNull($cache->getItem($cacheKey)->get()); + $this->assertFalse($inUse[0]->exists()); + } + + private function assertPoolCounts(CacheItemPoolInterface $cache, $key, $queue, $inUse, $toCreate) + { + $item = $cache->getItem($key)->get(); + $this->assertCount($queue, $item['queue'], 'Sessions In Queue'); + $this->assertCount($inUse, $item['inUse'], 'Sessions In Use'); + $this->assertCount($toCreate, $item['toCreate'], 'Sessions To Create'); + } +} diff --git a/Spanner/tests/Unit/Batch/BatchClientTest.php b/Spanner/tests/Unit/Batch/BatchClientTest.php index 82fff1c767aa..0ea1d83d2e47 100644 --- a/Spanner/tests/Unit/Batch/BatchClientTest.php +++ b/Spanner/tests/Unit/Batch/BatchClientTest.php @@ -90,9 +90,6 @@ public function testSnapshot() $this->assertInstanceOf(BatchSnapshot::class, $snapshot); } - /** - * @group foo - */ public function testSnapshotFromString() { $time = time(); diff --git a/Spanner/tests/Unit/Connection/GrpcTest.php b/Spanner/tests/Unit/Connection/GrpcTest.php index fb18328204e7..5303f88035ab 100644 --- a/Spanner/tests/Unit/Connection/GrpcTest.php +++ b/Spanner/tests/Unit/Connection/GrpcTest.php @@ -344,6 +344,26 @@ public function testCreateSessionAsync() $this->assertInstanceOf(PromiseInterface::class, $promise); } + public function testBatchCreateSessions() + { + $count = 10; + $template = [ + 'labels' => [ + 'foo' => 'bar' + ] + ]; + + $this->assertCallCorrect('batchCreateSessions', [ + 'database' => self::DATABASE, + 'sessionCount' => $count, + 'sessionTemplate' => $template + ], $this->expectResourceHeader(self::DATABASE, [ + self::DATABASE, $count, [ + 'sessionTemplate' => $this->serializer->decodeMessage(new Session, $template) + ] + ])); + } + public function testGetSession() { $this->assertCallCorrect('getSession', [ diff --git a/Spanner/tests/Unit/Session/CacheSessionPoolTest.php b/Spanner/tests/Unit/Session/CacheSessionPoolTest.php index bd8511a2174d..8c8eab3a9d5e 100644 --- a/Spanner/tests/Unit/Session/CacheSessionPoolTest.php +++ b/Spanner/tests/Unit/Session/CacheSessionPoolTest.php @@ -25,7 +25,6 @@ use Google\Cloud\Spanner\Session\Session; use Google\Cloud\Core\Testing\GrpcTestTrait; use GuzzleHttp\Promise\PromiseInterface; -use GuzzleHttp\Promise\RejectedPromise; use Psr\Cache\CacheItemPoolInterface; use Prophecy\Argument; use Prophecy\Argument\ArgumentsWildcard; @@ -321,7 +320,7 @@ public function testWarmup() $this->getCacheItemPool(), ['minSessions' => $expectedCreationCount] ); - $pool->setDatabase($this->getDatabase()); + $pool->setDatabase($this->getDatabase(false, false, 5)); $response = $pool->warmup(); $this->assertEquals($expectedCreationCount, $response); @@ -669,7 +668,7 @@ public function acquireDataProvider() ]; } - private function getDatabase($shouldCreateFails = false, $willDeleteSessions = false) + private function getDatabase($shouldCreateFails = false, $willDeleteSessions = false, $expectedCreateCalls = null) { $database = $this->prophesize(Database::class); $session = $this->prophesize(Session::class); @@ -708,20 +707,33 @@ private function getDatabase($shouldCreateFails = false, $willDeleteSessions = f ->willReturn(self::DATABASE_NAME); $createdSession = $this->prophesize(\Google\Cloud\Spanner\V1\Session::class); - $connection->createSessionAsync(Argument::any()) - ->will(function ($args, $mock, $method) use ($createdSession, $shouldCreateFails) { - if ($shouldCreateFails) { - return new RejectedPromise("error"); - } - - $methodCalls = $mock->findProphecyMethodCalls( - $method->getMethodName(), - new ArgumentsWildcard($args) - ); - - $createdSession->getName()->willReturn('session' . count($methodCalls)); - return $createdSession->reveal(); - }); + $createRes = function ($args, $mock, $method) use ($createdSession, $shouldCreateFails) { + if ($shouldCreateFails) { + throw new \Exception("error"); + } + + $methodCalls = $mock->findProphecyMethodCalls( + $method->getMethodName(), + new ArgumentsWildcard($args) + ); + + return [ + 'session' => [ + [ + 'name' => 'session' . count($methodCalls) + ] + ] + ]; + }; + + if ($expectedCreateCalls) { + $connection->batchCreateSessions(Argument::any()) + ->shouldBeCalledTimes($expectedCreateCalls) + ->will($createRes); + } else { + $connection->batchCreateSessions(Argument::any()) + ->will($createRes); + } return $database->reveal(); } diff --git a/Spanner/tests/Unit/ValueMapperTest.php b/Spanner/tests/Unit/ValueMapperTest.php index fdcb2e85944c..a5df1932213f 100644 --- a/Spanner/tests/Unit/ValueMapperTest.php +++ b/Spanner/tests/Unit/ValueMapperTest.php @@ -643,7 +643,6 @@ public function testFormatParamsForExecuteSqlStdClassMissingDefinition() /** * @dataProvider simpleTypeValues - * @group foo */ public function testEncodeValuesAsSimpleType($value, $expected = null) {