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

Consistent read in find? #38

Closed
cobbr2 opened this issue Oct 4, 2016 · 14 comments
Closed

Consistent read in find? #38

cobbr2 opened this issue Oct 4, 2016 · 14 comments

Comments

@cobbr2
Copy link

cobbr2 commented Oct 4, 2016

As far as I can tell, the request_options gets nothing from the hash we pass in, so there's no way to specify that a find should be consistent. This definitely causes problems in our test environment, where we get "heisenspecs": tests that sometimes pass and sometimes fail, depending on when "eventually" happens. I'd be happy to work on a patch... but given that the arguments to find are a hash, it would mean introducing reserved words (consistent_read) in a place that doesn't obviously have to worry about them now.

@awood45 awood45 added the bug label Oct 4, 2016
@awood45
Copy link
Member

awood45 commented Oct 4, 2016

I'd be interested in a patch and a reproducing example (accepting that the reproducing example may not always reproduce), since this sounds like a potential bug.

@awood45
Copy link
Member

awood45 commented Oct 4, 2016

If I'm understanding correctly, you're running many threads, concurrently calling find with different key parameters, and occasionally getting incorrect responses, such as the wrong return object?

@cobbr2
Copy link
Author

cobbr2 commented Oct 4, 2016

I'm never getting the wrong return object, but we're often getting no object at all, even though the write has returned. Many of our tests assume read-after-write consistency: do x and y and expect to read z (sometimes through a GSI). We do run many tests in parallel, but in principle they never try to read each other's data. All the keys are randomized from a very large pool (10^8 or larger, depending on the test), and the number of tests (10 or so) is way too small to run into the birthday paradox.

@cobbr2
Copy link
Author

cobbr2 commented Oct 4, 2016

Oh -- and I'm aware that we can't ever assume read-after-write from a GSI. The tests blowing up today were doing writes or updates followed by find. They've become more stable now that I've written those using the query interface and specified consistent_read: true.

@awood45
Copy link
Member

awood45 commented Oct 4, 2016

That's a good point actually, especially for users of eventual consistency tables (even if that's not necessarily your use case).

@awood45
Copy link
Member

awood45 commented Oct 4, 2016

I'm suspecting from the correspondence so far (correct me if I'm misunderstanding) that this may be a service behavior/missing feature issue rather than a bug in the find implementation.

@cobbr2
Copy link
Author

cobbr2 commented Oct 4, 2016

I'd claim its a missing feature. The find method signature is:

class.find( attribute: value, attribute: value,... )

It will ignore attributes that aren't keys, but it does not extract consistent_read: true and add that to the underlying get_record call. I.e., it's not added to the request_opts in

resp = dynamodb_client.get_item(request_opts)

The question is: if we wanted to allow for that, this would preclude an attribute named consistent_read from being a hash or range key. I.e., my call:

  Foobar.find( id: 3, consistent_read: true )

in the case where Foobar has a sort_key of :consistent_read, would imply both a search on consistent_read and that the search was done using consistent_read semantics. Is that a problem?

@awood45
Copy link
Member

awood45 commented Oct 4, 2016

That's what I'm wrestling with. It would preclude that attribute.

Now, you can work around that with the :database_attribute_name parameter when creating an attribute, and using a different name in your code. But, if anyone is using a key named :consistent_read, this will break them.

I could add this as a minor version bump potentially, or try to find another way to expose additional #get_item parameters.

@cobbr2
Copy link
Author

cobbr2 commented Oct 4, 2016

Or maybe just add find_consistently?

@awood45
Copy link
Member

awood45 commented Oct 4, 2016

That's another alternative, certainly. But then the question is if any of the other parameters will also be important. I don't want to add functions more than once if I can avoid it :)

@cobbr2
Copy link
Author

cobbr2 commented Oct 4, 2016

Point taken. I kind of like dynamoid's collection-modifier approach on where (its equivalent of query, I think). But that'd be a big lift on find. On their find, it looks like they assume positional arguments (array entries) for the id & range key. That allows them to treat all hash argument pairs as options.

@awood45
Copy link
Member

awood45 commented Oct 4, 2016

Yup, I hear you. We're working on some proposals in the team, any solution would accept any of the additional #get_object parameters, including any future ones that don't exist yet (much like we do in many other methods).

@mariokostelac
Copy link

@awood45 do you have any ETA for version with consistent reads support? Thanks :)

@awood45
Copy link
Member

awood45 commented Dec 7, 2016

I'm seeing this as the biggest pain point right now given the PR and multiple people asking. Going to try and look at a new method this weekend. Open to quick input regarding what to call a new method:

  • #find_with_options
  • #find_with_opts
  • Something else?

The relative usage would probably be as follows:

item = Model.find(hk_attr: "h_value", rk_attr: "r_value")
item = Model.find_with_opts(
  key: {
    hk_attr: "h_value",
    rk_attr: "r_value"
  },
  consistent_read: true
)

Open to feedback on approach as well.

awood45 added a commit that referenced this issue Dec 11, 2016
This method allows for a variant of the `#find` operation which will let
users pass through other options to the underlying
`Aws::DynamoDB::Client#get_item` request.

Resolves #38, and relates to #46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants