Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent nested transactions #533

Merged
merged 3 commits into from
Jun 28, 2017
Merged

Prevent nested transactions #533

merged 3 commits into from
Jun 28, 2017

Conversation

jdpedrie
Copy link
Contributor

@jdpedrie jdpedrie commented Jun 12, 2017

Closes #528.

Please note that while this change prevents calling Database::runTransaction() inside Database::runTransaction(), it only works on a single Database instance. There is no protection against a user creating a new instance of the client and injecting it into the runTransaction callback. If we must protect against this, it will require additional discussion and work, and would be much more challenging.

/cc @vkedia.

edit: Examples of what I explained above:

use Google\Cloud\Spanner\SpannerClient;

$spanner = new SpannerClient();
$db = $spanner->connect('instance', 'database');

$db->runTransaction(function ($t) use ($db) {
    // Exception thrown here.
    $db->runTransaction(function (){});
});
use Google\Cloud\Spanner\SpannerClient;

$spanner = new SpannerClient();
$db = $spanner->connect('instance', 'database');
$db2 = $spanner->connect('instance', 'database');

$db->runTransaction(function ($t) use ($db2) {
    // No exception will be thrown.
    $db2->runTransaction(function () {});
});

@jdpedrie jdpedrie added the api: spanner Issues related to the Spanner API. label Jun 12, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 12, 2017
@vkedia
Copy link

vkedia commented Jun 21, 2017

I think its ok to prevent this just within a single client.

*/
public function runTransaction(callable $operation, array $options = [])
{
if ($this->isRunningTransaction) {

This comment was marked as spam.

This comment was marked as spam.

@@ -511,6 +516,10 @@ public function iam()
*/
public function snapshot(array $options = [])

This comment was marked as spam.

@jdpedrie jdpedrie merged commit b3ff3e8 into googleapis:master Jun 28, 2017
@jdpedrie jdpedrie deleted the spanner-nested-transactions branch June 28, 2017 18:12
@jdpedrie jdpedrie mentioned this pull request Jun 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants