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

Record Instance Update #59

Open
IbottaBaier opened this issue Apr 4, 2017 · 4 comments
Open

Record Instance Update #59

IbottaBaier opened this issue Apr 4, 2017 · 4 comments
Labels
feature-request A feature should be added or improved.

Comments

@IbottaBaier
Copy link

IbottaBaier commented Apr 4, 2017

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
Copy link
Author

IbottaBaier commented Apr 11, 2017

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
Copy link
Member

awood45 commented Apr 14, 2017

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
Copy link
Author

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
Copy link
Contributor

mullermp commented Apr 12, 2022

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

update()
find()

@mullermp mullermp added the feature-request A feature should be added or improved. label Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

4 participants