aws / aws-sdk-ruby-record

Official repository for the aws-record gem, an abstraction for Amazon DynamoDB.
Apache License 2.0
318 stars 41 forks source link

Record Instance Update #59

Open IbottaBaier opened 7 years ago

IbottaBaier commented 7 years ago

Right now save (or save!) encapsulates the selection between a put and an update, with the ability to force put. I recently wanted a "find or create" method, and naively did:

record = MyRecord.find(key: 123)
record ||= MyRecord.new(key: 123)
...
record.save! #=> Conditional write exception

I end up receiving the conditional write exception because some other process is trying to create the same record.

To fix this, I moved to using an update:

MyRecord.update(key: 123, value: 'some new data')

However, we have a GSI which is dynamically generated from the values which update never takes into account (see issue #58). Thus, the preferred solution would be:

MyRecord.new(key: 123, value: 'some new data').upsert!

To achieve this I copied the else statement from ItemOperations#_perform_save which builds an update request for dirty columns and creates the record if it does not already exist; hence upsert.

IbottaBaier commented 7 years ago

I ended up moving to an upsert which returns the final object... sadly was a bit hacky because Record.update! does not accept request parameters, all parameters are treated as attributes for the record.

To handle this I created a SimpleDelegator which delegates an Aws::DynamoDB::Client and provides additional functionality:

  class InterceptClient < SimpleDelegator

    attr_accessor :update_item_params

    # inject additional parameters for update requests
    def update_item(request)
      super(request.merge(update_item_params))
    end
  end

I then created a new_client function for generating clients (mainly exposed to be overridden in test, if necessary):

      # Spawns a new client with necessary request parameter overrides.
      def self.new_client
        client = InterceptClient.new(Aws::DynamoDB::Client.new(stub_responses: Rails.env.test?))
        client.update_item_params = { return_values: 'ALL_NEW' }
        client
      end

This is severely limiting due to the fact that the class and all instances share the same client - this means ALL updates must return 'ALL_NEW' attributes; there is no way to (safely) change that value in a multi-threaded environment (using Shoryuken for processing SQS messages and reading/writing Dynamo).

awood45 commented 7 years ago

I see what you're getting at (and I think a V2 rev of this library may be in the works to address some API concerns such as these), but I'm curious for some more details of what you're trying to do:

I end up receiving the conditional write exception because some other process is trying to create the same record.

Is that a problem? The idea of conditional writes was that if multiple processes are trying to make the same record, you would want to halt, or use :force to treat it as a hash table. Are you trying to write different attributes to the same key, and that's where the sync problem is coming from?

IbottaBaier commented 7 years ago

In one of my use cases forcing would be the appropriate thing to do because we were completely overriding records; in another use case we only wanted to update the attributes provided. Update and returning the record came into play because I want to provide an update to one column while reading another column at the same time.

Maybe the functional gap is that Record.update returns a raw DynamoDB Client response rather than a record object. My upsert! function creates a record, performs an update based on the record's attributes, and constructs and returns a post-update record from the API response.

mullermp commented 2 years ago

Adding as a feature request to support client options for self.update and other variants.

update() find()