Skip to content

Commit

Permalink
Fixes soundcloud#107 - automatic retry on deadlock
Browse files Browse the repository at this point in the history
  • Loading branch information
Aswin Anand authored and Will Barrett committed Apr 29, 2019
1 parent 5090712 commit 3d67d5c
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 1 deletion.
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,23 @@ Ruby:
Lhm.cleanup(:run, until: Time.now - 86400)
```

## Retry on deadlock

LHM supports automatic retry on deadlock. By default it retries 10 times with
a 10 second delay between each retry. It can be customized using the following
options:

1. `retry_on_deadlock` - true / false (true by default)
2. `retry_attempts` - Number of retries on deadlock (10 by default)
3. `retry_wait_time` - Number of seconds to wait between each retry
(10 seconds by default)

```ruby
Lhm.change_table :users, :retry_on_deadlock => true, :retry_attempts => 5 do |m|
m.add_column :arbitrary, "INT(12)"
end
```

## Contributing

First, get set up for local development:
Expand Down
67 changes: 66 additions & 1 deletion lib/lhm/chunker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,48 @@ def initialize(migration, connection = nil, options = {})
@start = options[:start] || select_start
@limit = options[:limit] || select_limit
@printer = options[:printer] || Printer::Percentage.new
@retry_on_deadlock = retry_on_deadlock(options.with_indifferent_access)
@retry_attempts = retry_attempts(options.with_indifferent_access)
@retry_wait_time = retry_wait_time(options.with_indifferent_access)
end

def execute
return unless @start && @limit

retries = 0

@next_to_insert = @start
while @next_to_insert <= @limit
stride = @throttler.stride
affected_rows = @connection.update(copy(bottom, top(stride)))

begin
affected_rows = @connection.update(copy(bottom, top(stride)))
rescue ActiveRecord::StatementInvalid => err
if err.message.downcase.index('deadlock').nil?
raise
return

This comment has been minimized.

Copy link
@jasonrosendale

jasonrosendale Apr 29, 2019

The return doesn't do anything here -- is it there to make it easier to follow the logic branches?

This comment has been minimized.

Copy link
@willbarrett

willbarrett Apr 29, 2019

I'm not sure - we might need to ask the original author.

end

if !@retry_on_deadlock
raise
return
end

retries = retries + 1
if retries == (@retry_attempts + 1)

This comment has been minimized.

Copy link
@jasonrosendale

jasonrosendale Apr 29, 2019

I've seen a few clients (e.g. the ruby kafka client) where users are expected to set "retry_attempts" to -1 in order to disable retries. :) If we're just using this branch for this week's sprint, then it's NBD -- we'll just make sure to leave retry_attempts at its default value

This comment has been minimized.

Copy link
@willbarrett

willbarrett Apr 29, 2019

I think the goal is to use this for this week's sprint to unblock us, but also eventually to clean it up to the point where it could be a contribution to soundcloud/lhm. This was a cherry-pick from another repository. I agree that we should think about the right settings here when that comes around.

puts "Crossed #{@retry_attempts} attempts. Raising exception ..."
raise
return
else
puts "Caught exception: #{err.message}."
print "Attempt #{retries} of #{@retry_attempts} "
puts "after sleeping for #{@retry_wait_time} seconds ..."
sleep @retry_wait_time
next
end
end

retries = 0

if @throttler && affected_rows > 0
@throttler.run
Expand Down Expand Up @@ -108,5 +142,36 @@ def validate
error('impossible chunk options (limit must be greater than start)')
end
end

def retry_on_deadlock(options)
if options.has_key?(:retry_on_deadlock) &&
( options[:retry_on_deadlock].is_a?(TrueClass) ||
options[:retry_on_deadlock].is_a?(FalseClass) )

return options[:retry_on_deadlock]
end

return true
end

def retry_attempts(options)
if options.has_key?(:retry_attempts) &&
options[:retry_attempts].is_a?(Numeric)

return options[:retry_attempts]
end

return 10
end

def retry_wait_time(options)
if options.has_key?(:retry_wait_time) &&
options[:retry_wait_time].is_a?(Numeric)

return options[:retry_wait_time]
end

return 10
end
end
end

0 comments on commit 3d67d5c

Please sign in to comment.