DynamoDB Transactions

[OmerAyubi]

Hey does Dyanmoid support DynamoDB transactions:
https://aws.amazon.com/blogs/aws/new-amazon-dynamodb-transactions/

[andrykonchin]

Dynamoid doesn't support transactions right now. The implementation is in progress (#468) and may be released in the next minor version.

[OmerAyubi]

Hey @andrykonchin thanks for the follow up! Question -> when do you think this will be released?

[andrykonchin]

The next release will be in April or May and I suppose that transaction api will be released as well.

[OmerAyubi]

got it, thanks!

[ckhsponge]

I just submitted a PR with a working proposal for a transact API that can span tables. @andrykonchin, feedback would be appreciated before I proceed.
#688
https://github.com/ckhsponge/dynamoid/blob/transact/README_transact.md

Are there instructions on running the tests? I'm seeing 41 errors when I checkout 3.9.0 although no errors on 3.8.0.

Thanks!

[andrykonchin]

The main question is interface of transactions.

There are two ways of thinking about the interface:

• it resembles existing Dynamoid::Document interface and has the same methods to do the same operations (but in transaction) e.g. update attributes, save, create, etc (I've already proposed such example here)
• new interface (like in the proposed prototype)

I don't have strong opinions about this but would like to at least enumerate pros and cons.

New interface:

• it makes us free from supporting all the features of the existing one
• we can avoid mistakes made in the current interface (that are not so easy to fix as far at it would be breaking change)

Copying existing interface:

• it seems to me that the main benefit is there is no new interface:
  • no need to get familiar with new API (actually a minor issue - there may be only a few new methods)
  • no need to change existing code significantly in case of switching to/from transactions (probably will be done only once after switching to a Dynamoid version with transactions support).

Of course I am talking mainly about interface of modifying items, not reading, because even current reading interface is pretty simple (just a few finders - find/find_all).

[andrykonchin]

The proposed interface is a new interface and very similar to what another DynamoDB ORM provides (link) and probably most of ORMs/libraries in other languages I've seen (in JavaScript and Python). It isn't bad of course, it's just a remark.

I would admit that it's high-level and operates on the model class/model instance level. On the other hand I would note some made decisions (at least what I've noticed looking at the readme document, by the way it's cool that it's present!):

• operations is an Array, a data structure, and a client needs to build it explicitly. In case of conditions logic, scattering application/business logic among several methods - they operate (pass/return) on the level of Arrays and Hashes. It seems too low-level for ORM. It may work pretty good for simple and straightforward logic, but may become a problem in other cases.
• (minor issue) relying on keys (but not on an parameter position) may look too long and wordy, I mean model/model_class/attributes. It provides flexibility in exchange for length/readability.
• supporting update_expression gives a client full control and flexibility but it seems again too low-level for ORM. It's probably makes sense for a client to use adapter's #transact_write_items method instead so it should be as handy as possible still being low-level.

What do you think?

[ckhsponge]

Thanks for reviewing!

I can see the benefit of using a style like ActiveRecord transactions. Using some sort of collection of operations is more straightforward for the reasons you mention like not needing to support everything. We can't just grab existing SDK inputs, combine them and throw them into a transaction because the formats are different e.g. updating an item uses :attribute_updates but in #transact_write_items an :update_expression is required. Transforming existing SDK inputs or starting from scratch is a possibility.

As opposed to Arrays and Hashes, I'll try creating a transaction builder that can be added to and then executed.

[andrykonchin]

The POC PR was updated (bd91a16) and I find provided examples thoughts provoking:

I like flexibility of running transaction with block and without it (with explicit committing), for instance:

txn = Dynamoid::Transaction.new
txt.save(user)
# ...
txn.commit

In case of copying existing interface it might be an option just to support an additional transaction argument in the existing model methods:

txn = Dynamoid::Transaction.new
user.save(txt)
user.update_attributes(txt, age: 2)

But now it isn't obvious what such call should return - there is no actual result now and it will be available only after "committing" the transaction. Should it be a Promise? Sounds too complex. Actually promises could be an option for the transactional reading interface.

When transaction is used without block - should transaction object be explicit or implicit? We could store it in thread locals until a transaction is committed and it could be used under the hood:

Dynamoid::Transaction.start # Thread.current["transaction"] = Dynamoid::Transaction.new
# ... any interface that doesn't involve passing/specifying the transaction object explicitly
Dynamoid::Transaction.commit # or abort

Regarding returned value, model validation and callbacks. When validation and callbacks should be called - immediately when some update, save etc method is called or at the end right before "committing" transaction? Callbacks definitely should be called at the end but validation could be called earlier (and returned as result).

If validation failed (and it isn't save! or create! methods that should raise exception and terminate a transaction) or callback cancels/halts callbacks chain execution (with throw :abort) we should probably just skip this model and continue processing other ones without terminating the transaction.

Personally I still prefer a tread-off between simplicity, readability and comfort that was already mentioned in (#468 (comment)):

Dynamoid::Document.transaction do |txn|
  txn.create(User, attributes)
  txn.update(User, attributes)
  txn.update_fields(User, attributes, conditions)
  txn.upsert(User, attributes)
  txn.delete(User, id)
end

Explicit transaction object, its explicit usage, the interface is similar to the existing one but not coupled with existing model methods.

As an idea to make arguments more readable it might make sense to separate a target (model class or model instance) and action arguments (attributes, options, etc), e.g. this way:

txn.with(User).create(attributes)
txn.with(user).save()
txn.with(user).update_attributes({})

Some other thoughts:

• Using raw inputs - I would say it could be useful, but would postpone its implementing until somebody explicitly asks for it and provides clear use case
• Skipping callbacks and validations - I don't know whether it would be useful. I would discuss it separately as a feature request. Again if it's asked explicitly and a use case is provided. Some methods don't run callbacks (or shouldn't), e.g. #delete, increment etc. At saving validation could be skipped with save(validate: false)
• Using an ADD expression - it's already supported with .inc and #update methods
• using instance_eval means self is replaced. So any methods of a class where transaction is executed will not be available inside block what could be an issue. I would consider passing transaction object as a block parameter explicitly:

Dynamoid::Transaction.execute do |txn|
  txn.save(model)
end

The issue with instance_eval:

class A
  def bar; 1 end
  def test
    "".instance_eval { bar }
  end
end

A.new.test
# (irb):18:in `block in test': undefined local variable or method `bar' for "":String (NameError)

[ckhsponge]

Thanks for the detailed feedback! I like your ideas about having an explicit txn object and being able to use it outside of a block. That could be useful.

Storing the transaction on the thread doesn't seem feasible to me at the moment. The existing actions don't all translate exactly to something that can be in a transaction and I'm also not modifying existing code too much. It would be heavy work.

I hadn't considered allowing transactions to partially persist if a model validation fails and there is no ! on it. Currently no callbacks are run until they are all run. If we want to allow a validation to fail and continue with other actions then we will want to allow checking for the save success so before callbacks will have to be run earlier and one by one. Since we are doing this in a transaction the after callbacks must still all be run together after the persist. Actually, we could do the after callbacks after adding to the transaction but before the actual persist.