Introduce Google Cloud Spanner #438

dwsupplee · 2017-04-07T17:32:23Z

Introducing.... Google Cloud Spanner support for PHP!

This PR is ready for review and feedback with a small caveat, there are a small handful of tests which need to be updated yet.

A few items worth noting for discussion:

A request from the Spanner team for the session pool was to prime transactions - we have left this out of our current cache based session pool implementation. For every transaction we expend we would need to immediately replace it with a fresh one - given that the pool is managed by each request flowing through the code this wouldn't provide any optimizations unfortunately. Ideally, in time, we will add other session pool implementations that can run as a process alongside the application code and gain the benefits of priming the transactions.
The cache based session pool relies on a locking mechanism in order to prevent concurrent requests from colliding. Symfony just recently introduced a lock component which covers the use cases we were already attempting to implement. As locking can can be complex, it seems sensible to use a component which has had lots of eyes on it and serious consideration taken into it's design and testing. The downside here, is that the component requires PHP > 5.5.9 (where we currently require > 5.5) and it will not be officially released until May. I am okay with bumping our required version of PHP up slightly, as it is highly unlikely any users are running versions between 5.5 and 5.5.9.. Thoughts are certainly appreciated, however. - Update The symfony lock component has been postponed to Symfony 3.4. As is such, it is no longer safe to rely on it being available. We will be using a dev-master commit from the symfony lock component during the early stages of alpha with plans to transition to our own locking mechanisms soon thereafter.

/cc @vkedia @tmatsuo

https://github.com/dwsupplee/gcloud-php/tree/spanner/src/Spanner

TODO

Remove $options.transaction from Database::runTransaction(). (jdp)
Support single-use transactions in Database::transaction(), runTransaction(). (jdp)
Make Database mutation methods use abort retry. (jdp)
Add static method to Date to create with year, month, day
Fix labels on updateInstance (possible race condition) (jdp)
Buffer rows in Result until resumeToken is received (dws)
P1 system test coverage to spec. (both)

jdpedrie

couple odds and ends. still need to take more time studying the pool implementation.

src/Core/PhpArray.php

+     * }
     */
-    public function __construct(array $customFilters = [])
+    public function __construct(array $config = [])


src/Logging/Connection/Grpc.php

-            'outputVersionFormat' => function ($v) {
-                return self::$versionFormatMap[$v];
-            }
+            'customFilters' => [


src/PubSub/Connection/Grpc.php

-        $this->codec = new PhpArray(['publishTime' => function ($v) {
-            return $this->formatTimestampFromApi($v);
-        }]);
+        $this->codec = new PhpArray([


src/Spanner/Connection/Grpc.php

+    public function __construct(array $config = [])
+    {
+        $this->codec = new PhpArray([
+            'customFilters' => [


vkedia · 2017-04-07T23:01:12Z

Thanks for sending this. Really excited to see this!!
I am out of office till 15th. I will take a look after that.

tmatsuo · 2017-04-11T03:43:02Z

@dwsupplee Thanks!
I don't see any major issues requiring 5.5.9

src/Spanner/Session/CacheSessionPool.php

+     * @var array
+     */
+    private static $defaultConfig = [
+        'maxSessions' => PHP_INT_MAX,


src/Spanner/Session/CacheSessionPool.php

+    {
+        $sessions = [];
+
+        for ($i = 0; $i < $count; $i++) {


vkedia

Made some headway into the review.

docs/contents/cloud-spanner.json

@@ -0,0 +1,44 @@
+{
+    "title": "Spanner",


src/Core/Exception/AbortedException.php

+            return false;
+        });
+
+        $delay = $metadata[0]['retryDelay'];


src/Core/Exception/AbortedException.php

+        });
+
+        $delay = $metadata[0]['retryDelay'];
+        if (!isset($delay['seconds'])) {


src/Core/LongRunning/LongRunningOperation.php

+     * @param array $options [optional] Configuration options.
+     * @return mixed|null
+     */
+    public function result(array $options = [])


src/Spanner/Configuration.php

+ * @see https://cloud.google.com/spanner/docs/reference/rpc/google.spanner.admin.instance.v1#instanceconfig InstanceConfig
+ * @codingStandardsIgnoreEnd
+ */
+class Configuration


src/Spanner/Database.php

+    }
+
+    /**
+     * Create and return a new read/write Transaction.


src/Spanner/Database.php

+     * @param array $options [optional] Configuration options.
+     * @return Timestamp The commit Timestamp.
+     */
+    public function insertBatch($table, array $dataSet, array $options = [])


src/Spanner/Database.php

+    /**
+     * Update a row.
+     *
+     * Only data which you wish to update need be included. You must provide


src/Spanner/Database.php

+        } finally {
+            $session->setExpiration();
+        }
+    }


src/Spanner/Database.php

+        } finally {
+            $session->setExpiration();
+        }
+    }


vkedia · 2017-05-04T07:26:02Z

Also do you guys handle null values everywhere? So params can be null (in that case how do you infer the type of the param?). Column values can be null, arrays can contain null.

jdpedrie · 2017-05-04T14:17:48Z

@vkedia, I'm looking into that now.

What is a column definition which would accept null values as array elements?

Column definition:

arrayField ARRAY<INT64> NOT NULL

Insert data:

$database->insert('tableName', [
    'arrayField' => [1, 2, null]
]);

Error:

FailedPreconditionException: Invalid value for column arrayField in table tableName: Expected INT64

That error isn't surprising, but I can't think of how one would add a null array element. The documentation says it is possible.

vkedia · 2017-05-05T20:56:36Z

@jdpedrie I believe all arrays can contain null elements. Can you try removing "NOT NULL" from there?

jdpedrie · 2017-05-05T21:02:45Z

@vkedia that did the trick. sorry for the noise.

vkedia · 2017-05-05T21:05:45Z

@jdpedrie Actually I am surprised that specifying "NOT NULL" also makes the elements be non nullable.

dwsupplee · 2017-05-05T23:55:30Z

@vkedia is there anything else we need to touch on besides #438 (comment)?

src/Spanner/Session/CacheSessionPool.php

+        'maxSessions' => PHP_INT_MAX,
+        'minSessions' => 1,
+        'shouldWaitForSession' => true,
+        'maxCyclesToWaitForSession' => 30,


src/Spanner/Session/CacheSessionPool.php

+     * @var array
+     */
+    private static $defaultConfig = [
+        'maxSessions' => PHP_INT_MAX,


src/Spanner/Session/CacheSessionPool.php

+    {
+        $sessions = [];
+
+        for ($i = 0; $i < $count; $i++) {


src/Spanner/Session/CacheSessionPool.php

+    {
+        foreach ($data['inUse'] as $key => $session) {
+            if ($session['lastActive'] + 1200 < $this->time()) {
+                unset($data['inUse'][$key]);


src/Spanner/Session/CacheSessionPool.php

+        $session = array_shift($data['queue']);
+
+        if ($session) {
+            if ($session['expiration'] - 60 < $this->time()) {


src/Spanner/Session/CacheSessionPool.php

+     */
+    private function purgeOrphanedToCreateItems(array &$data)
+    {
+        foreach ($data['toCreate'] as $key => $timestamp) {


src/Spanner/Session/CacheSessionPool.php

+     * @param array $config [optional] {
+     *     Configuration Options.
+     *
+     *     @type int $maxSessions The maximum number of sessions to store in the


src/Spanner/Session/SessionPoolInterface.php

+    public function release(Session $session);
+
+    /**
+     * Clear the session pool.


vkedia · 2017-05-09T06:06:34Z

I have added a bunch of comments to the SessionPool.

dwsupplee · 2017-05-09T17:00:16Z

Thanks for all the notes @vkedia! It's greatly appreciated.

Add system test coverage. Add more read system tests and fix KeyRange issues Spanner transaction system tests Add remaining p0 spanner system tests

doc fix address code review move symfony/lock from require to suggest

src/Spanner/composer.json

+    },
+    "suggest": {
+        "google/gax": "Required to support gRPC",
+        "google/proto-client-php": "Required to support gRPC"


vkedia

Couple of minor comments

src/Spanner/Session/CacheSessionPool.php

+ * on an interval that matches when you expect to see a decrease in traffic.
+ * This will help ensure you never run into issues where the Spanner backend is
+ * locked up after having met the maximum number of sessions assigned per node
+ * (The current maximum sessions per node is 10k).


src/Spanner/Session/CacheSessionPool.php

+
+        return [
+            'deleted' => count($toDelete) - count($failed),
+            'failed' => $failed


src/Spanner/Database.php

+    /**
+     * Insert a row.
+     *
+     * Mutations are committed in a single-use transaction.


dwsupplee added the api: spanner Issues related to the Spanner API. label Apr 7, 2017

googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Apr 7, 2017

dwsupplee added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Apr 7, 2017

dwsupplee force-pushed the spanner branch from 9ce7866 to 1e6bc1c Compare April 7, 2017 17:43

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Apr 7, 2017

dwsupplee added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Apr 7, 2017

jdpedrie reviewed Apr 7, 2017

View reviewed changes

jdpedrie force-pushed the spanner branch from 1e6bc1c to a9ebfee Compare April 10, 2017 14:10

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Apr 10, 2017

jdpedrie added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Apr 10, 2017

jdpedrie force-pushed the spanner branch from a9ebfee to 64aa2dd Compare April 10, 2017 14:16

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Apr 10, 2017

jdpedrie added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Apr 10, 2017

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Apr 10, 2017

jdpedrie added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Apr 10, 2017

tmatsuo reviewed Apr 11, 2017

View reviewed changes

vkedia suggested changes Apr 17, 2017

View reviewed changes

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Apr 18, 2017

jdpedrie force-pushed the spanner branch from 3b85a70 to 023180c Compare May 8, 2017 19:15

vkedia reviewed May 9, 2017

View reviewed changes

jdpedrie and others added 6 commits May 15, 2017 18:13

Introduce support for Cloud Spanner (googleapis#333)

462629f

introduce cache based session pool and streaming execute/read

6636b40

Implement remaining API, add system tests.

1629272

Buffer result sets and retry only with error code unavailable

097170f

Fix parameterization and value mapping

201e6b7

Add system test coverage. Add more read system tests and fix KeyRange issues Spanner transaction system tests Add remaining p0 spanner system tests

allow user determined row formatting

927b919

doc fix address code review move symfony/lock from require to suggest

bshaffer reviewed May 16, 2017

View reviewed changes

src/Spanner/composer.json Outdated

},

"suggest": {

"google/gax": "Required to support gRPC",

"google/proto-client-php": "Required to support gRPC"

This comment was marked as spam.

Sign in to view

bshaffer mentioned this pull request May 16, 2017

WIP - Add Spanner Quickstart GoogleCloudPlatform/php-docs-samples#369

Closed

Update README and Spanner composer.json

9becfc7

vkedia reviewed May 16, 2017

View reviewed changes

vkedia reviewed May 17, 2017

View reviewed changes

src/Spanner/Database.php

/**

* Insert a row.

*

* Mutations are committed in a single-use transaction.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dwsupplee added 2 commits May 17, 2017 14:12

update verbiage

b78a47e

small tweaks

57849d3

jdpedrie force-pushed the spanner branch from cd1593c to 57849d3 Compare May 17, 2017 18:15

jdpedrie and others added 3 commits May 17, 2017 16:32

Add resource-prefix header to outgoing spanner requests

22b994d

Merge branch 'master' into spanner

0386fab

inverse retry condition

33f743a

jdpedrie merged commit 33f743a into googleapis:master May 18, 2017

jdpedrie mentioned this pull request May 18, 2017

Prepare v0.30.0 #497

Merged

dwsupplee mentioned this pull request May 19, 2017

Spanner - Document how to use session pools and how they can impact performance #408

Closed

Introduce Google Cloud Spanner #438

Introduce Google Cloud Spanner #438

Uh oh!

Conversation

dwsupplee commented Apr 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

jdpedrie left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

vkedia commented Apr 7, 2017

Uh oh!

tmatsuo commented Apr 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

vkedia left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

dwsupplee commented Apr 7, 2017 •

edited

Loading

jdpedrie left a comment •

edited

Loading

tmatsuo commented Apr 11, 2017 •

edited

Loading

jdpedrie commented May 4, 2017 •

edited

Loading