Navigation Menu

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

Add transaction support for redis distributed #941

Merged
merged 3 commits into from Sep 7, 2020

Conversation

EiNSTeiN-
Copy link
Contributor

I am trying to use watch and multi with Redis::Distributed. My code looks like this:

r.watch("{uuid}:data", "{uuid}:version") do
  current = r.get("{uuid}:version")
  r.multi do
    r.set("{uuid}:data", new_data)
    r.set("{uuid}:version", current + 1)
  end
end

Given all keys can be tagged, they will all end up on the same node, so this should work with Redis::Distributed without issue. An additional assumption is that the caller will use the block syntax for watch which is thread safe, or will handle thread safety in another way outside of Redis::Distributed.

I implemented that by keeping a copy of the key tag or the key if the key itself isn't tagged, and ensure that all commands are issued with the same key or key tag until one of these things happen:

  1. exec, unwatch or discard is explicitly called
  2. exec is implicitly called at the end of the multi block
  3. unwatch is implicitly called when a StandardError is raised within a watch block

@EiNSTeiN- EiNSTeiN- changed the title transaction support for redis distributed Add transaction support for redis distributed Sep 4, 2020
@supercaracal
Copy link
Contributor

CI tests of the repository tend to be flaky now. These irrelevant failures are ignorable. They will be resolved by #930.

byroot
byroot previously requested changes Sep 7, 2020
begin
node.watch(*keys, &block)
rescue StandardError
@watch_key = nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not good enough, you want to reset @watch_key from an ensure block. Here you only reset it on error.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, actually scratch that, it seems watch do is not expected to unwatch... that's a very weird behavior :/

Copy link
Collaborator

@byroot byroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so ignore my previous comment. I wasn't so familiar with the watch interface. Turns out the unwatch has to be manual, sounds like a terrible API to me but hey...

@byroot byroot merged commit 69ce376 into redis:master Sep 7, 2020
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

Successfully merging this pull request may close these issues.

None yet

3 participants