Feature Request: Transactions

[zoe-edwards]

I know this news has been out less than 24 hours 🤣

Wanted to start the conversation around transactions and if it’s possible to integrate them with Eloquent’s transactions.

I don’t know Eloquent well enough to understand how it works with transactions. Do you think this might be possible to implement?

[baopham]

Hahah, I love this!

Maybe we can have something like:

DynamoDb::transaction(function () {
  // do stuff
});

[baopham]

I do want to get this feature in. Let's see if I can find sometime during the holiday to work on this.

[zoe-edwards]

Very happy to help if I can!

[asantibanez]

Thanks @baopham for your work! This package is awesome!

[baopham]

This is not official, but this is what I have in mind:

DynamoDb::transaction('write', function (DynamoDbTransaction $t) {
    $query = $t->beginTransactItem();

    $query->table('test')
        ->key(['id' => 'foo1'])
        // the optional condition expression that
        // must be satisfied for the operation to succeed
        ->satisfy('foo', 'bar')
        ->orSatisfy('foo', 'bar2') 
        ->delete();

    $query->table('test')
        ->key(['id' => 'foo3'])
        ->satisfy('foo', 'bar')
        ->put([
            'id' => 'foo3',
            'foo' => 'new bar'
        ]);

    $query = $t->beginTransactItem();

    $query->table('test')
        ->key(['id' => 'foo3'])
        ->satisfy('foo', 'bar')
        ->check();
});

DynamoDb::transaction('write', function (DynamoDbTransaction $t) {
    $query = $t->beginTransactItem();

    $query->for($model)
        // the optional condition expression that
        // must be satisfied for the operation to succeed
        ->satisfy('foo', 'bar') 
        ->orSatisfy('foo', 'bar2') 
        ->delete();

    $query->for($model)
        ->satisfy('foo', 'bar')
        ->put([
            'id' => 'foo3',
            'foo' => 'new bar'
        ]);

    $query = $t->beginTransactItem();

    $query->for($model)
        ->satisfy('foo', 'bar')
        ->check();
});

What do you think?

/cc @zoul0813

[zoul0813]

Will take a closer look this week, haven’t had time to read up on the latest announcements from re:Invent yet :(

With that said, syntax looks clean and intuitive though. May have more thoughts after I’ve read up on the new features though.

[zoul0813]

I've looked into it a bit, and I'm not sure I like the satisfy methods ... can these just be named where, orWhere, etc to be more natural/intuitive?

Can we have DynamoDBTransaction support a save and delete method ... so we can do something like

DynamoDb::transaction('write', function (DynamoDbTransaction $t) {
  $m1 = new Model([...]);
  $t->save($model);
  $m2 = new Model([...]);
  $t->save($m2);
  $m1->prop = $m2->prop;
  $t->update($m1);
});

As far as I understand, the TransactItems within a transactGetItems or transactWriteItems is just a collection of PutItem or GetItem calls, and we already support both of these outside of transactions. So we should be able to generate the necessary PutItem and GetItem "queries" using existing code in DynamoDbQueryBuilder ... but instead of executing it, we just call toDynamoDbQuery (or similar) to retrieve the object, and add it to a collection, then use that collection to populate the transact(Get|Write)Items calls for transaction(...).

We should also implement the attempts logic that exists in Eloquent, so we can retry the transaction more than once before a hard failure (check out Illuminate\Database\Concerns\ManagesTransactions). Retries can maybe have a config value that sets the 'retry wait', so developers can have the code wait Nms (250ms, 300ms, 1200ms, etc) before retries ... this wait period is something they should be able to calculate based on their tables provisioned capacity for read/writes).

We can also drop the 'write' parameter that is in your example. The first operation within a transaction can set the 'write' or 'get' flag, and we can throw an exception if the user attempts to change the format within the transaction... this way, our transaction method can follow the Eloquent pattern and just be transaction(Closure $closure, $attempts = 1) ... as we can't support read and writes within DynamoDB Transactions, we can throw an exception if the user performs a save and then attempts to read from that ... or vice versa. As an alternative, to make code more readable, the 'write|get' can be passed as a third parameter, after the $attempts, so our function still follows the Laravel/Eloquent pattern ... transaction(Closure $closure, $attempts = 1, $op = 'write')

// Just my "two cents" ...

[sahilsharma011]

@zoul0813 Since Illuminate\Database\Concerns\ManagesTransactions is not available below Laravel 5.4
Does that mean that this feature will not be available for Laravel < 5.4 ?

[zoul0813]

We’d have to implement all of that ourselves, so it would be available at any version the project supports.

I’m simply proposing we follow the way that laravel is doing it.

[baopham]

@zoul0813

The first operation within a transaction can set the 'write' or 'get' flag, and we can throw an exception if the user attempts to change the format within the transaction

Yes, that'd simplify things. That is a good idea.

As for your suggestion for using where, I'm not keen on re-using where as it is conceptually different. I want to have a clear distinction and not mix up the idea. Essentially, it's setting the condition for the operation to succeed, not setting condition to filter records.

Btw, if we use the Eloquent syntax for this, how would you translate the TransactItem ConditionCheck to Eloquent syntax?

[zoul0813]

@baopham as far as I understand it, the ConditionCheck is just a standard expression, but requires a Key... allowing you to check if the item with Key matches the ConditionExpression ... it should follow all of the regular condition syntax, so the code generated by DynamoDbQueryBuilder for a where (and it's equals) should work.

Perhaps something like this:

DynamoDb::transaction(function (DynamoDbTransaction $t) {
  $m1 = new Model([...]);
  $t->where('property', 'value')->find(id)->save($model);
  $m2 = new Model([...]);
  $t->where('prop', 'val')->andWhere('another', 'property')->find(id)->save($m2);
  $m1->prop = $m2->prop;
  $t->update($m1);
}, $MAX_ATTEMPTS, 'write');

Note, these are chained because they all return a reference to the DynamoDbTransaction ($t). If the chain is started by DynamoDbTransaction, then it begins a new "TransactItem", if it's being chained from the DynamoDbConditionCheckQueryBuilder then it's part of the same TransactItem request.

We can take the logic from DynamoDbQueryBuilder and break it out into a parent class ... then we can have a DynamoDbQueryBuilder class that acts as it does now, and a DynamoDbConditionCheckBuilder that also inherits the same core "query" logic with a few extra bits (and removing any invalid functionality...). This way, we can retain the ability to chain the logic, and build standard style queries using common syntax without having to rewrite the core code for building DynamoDb Expressions.

We could also make this more verbose by requiring the user to request a new TransactItem, such as $t->put()->where(...)->find(...)->save(...) or $t->update()->where(...)->find(...)->save(...), where the put/update/delete methods would create new "Put", "Update" or "Delete" requests

[baopham]

@zoul0813

Let's not talk about implementation details yet.

I think we are not talking about the same ConditionCheck. There is a separate ConditionCheck that doesn't have any operation bound to it, but rather a condition for the entire transaction.

ConditionCheck — Applies a condition to an item that is not being modified by the transaction. This structure specifies the primary key of the item to be checked, the name of the table where it resides, a condition expression that must be satisfied for the transaction to succeed, and a field indicating whether or not to retrieve the item's attributes if the condition is not met.
https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_TransactWriteItems.html

If we are sticking to Eloquent. We will need to invent our own conventions

DynamoDb::transaction(function () {
  $otherModel->mustSatisfy('foo', 'bar')->orReturn();

  $model1->mustSatisfy('foo', 'bar')->update(['foo' => 'bar2']);

  $model2->mustSatisfy('foo', 'bar')->delete();
});

Or:

DynamoDb::transaction(function () {
  $otherModel->mustHave('foo', 'bar')->orReturn();

  $model1->mustHave('foo', 'bar')->update(['foo' => 'bar2']);

  $model2->mustHave('foo', 'bar')->delete();
});

Or sticking to Transaction object to avoid global flag for transaction (my preferred way) and just some modifications away from your proposal:

DynamoDb::transaction(function ($t) {
  $t->for(OtherModel::class)->find('id')->mustHave('foo', 'bar')->orReturn();

  // I prefer this because we don't suggest the user to make a separate query just to instantiate the Model1
  $t->for(Model1::class)->find('id')->mustHave('foo', 'bar')->update(['foo' => 'bar2']);

  $t->for(Model2::class)->find('id')->mustHave('foo', 'bar')->delete();
});

[baopham]

Wait, I missed addressing a few bits of your comment:

I think I misread the docs, actually TransactItems is just a list of ConditionCheck or Get or Put or Update object, not a group of operations (https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_TransactWriteItems.html#DDB-TransactWriteItems-request-TransactItems). So we don't need to have to worry about:

Note, these are chained because they all return a reference to the DynamoDbTransaction ($t). If the chain is started by DynamoDbTransaction, then it begins a new "TransactItem", if it's being chained from the DynamoDbConditionCheckQueryBuilder then it's part of the same TransactItem request.

So essentially anything in the closure will be grouped in TransactItems.

[zoul0813]

Actually, I misread the docs myself ... I thought the TransactWriteItem object was a combination of ConditionCheck and Put/Delete/Update ... looks like it's a FIFO style queue and TransactWriteItem only contains one of either Condition, Put, Delete or Update ... and if any of the TransactWriteItems fail, the transaction is rolled back...

I like the idea of creating a Transaction object (preferred way above), and chaining everything off of that.

If we implement it so that a call that comes directly off of the (Transaction) $t object starts a new TransactItem and adds it to the TransactItems array ... it would eliminate the need for developers to start/stop TransactItems explicitly.

$t->for(Model::class)->find('id')->mustHave('foo', 'bar');
$t->for(Model2::class)->save($model2);
$t->for(Model3::class)->delete($model3);

Would create three TransactWriteItem objects, one ConditionCheck, one Put/Update (determined by whether or not it is a 'new' or 'existing' model), and one Delete ... processed in that order?