tchandy/octopus — pull request #63 by austinmills (2011-12-28 20:09:18+00:00)
Title: Round-robin through slaves_list in a non-destructive manner

This pull request is the result of some errors we encountered in our production system which uses octopus to talk to a master and a single slave in fully_replicated mode. We would occasionally see a server start throwing errors like this:

NoMethodError: undefined method `to_sym' for nil:NilClass

…le/ruby/1.8/gems/ar-octopus-0.3.4/lib/octopus/proxy.rb: 171:in `send_queries_to_selected_slave'
…le/ruby/1.8/gems/ar-octopus-0.3.4/lib/octopus/proxy.rb: 140:in `method_missing'
... (our code from here down)

(We're using 0.3.4 but 0.4.0 is the same for the purpose of this discussion)

Once it started throwing this error, it would continue to throw it until the app was restarted. From looking at the line of code, it would seem to indicate that somehow our slaves_list (which previously had just one entry) had become empty. After a lot of investigation, we eventually tracked it down to this:

To cycle through the slaves, we pull the next slave from the front of the list, and then add it to the end of the list:
self.current_shard = @slaves_list.shift.to_sym
@slaves_list << self.current_shard

This is fine, generally. However, in our application, this activity is happening during an operation for which we've set a timeout. When that timeout hits, a Timeout::Error will be raised wherever the current thread of execution happens to be. Specifically, it could be raised between the time when the slave is pulled from the list and when it's put back on the list -- which would result in that slave forever being gone from the slaves_list. In an app that had a number of slaves, having one fewer would just result in the requests being distributed unevenly across the other slaves, but in our app with one slave it was immediately noticeable.

This change modifies the slave selection process to use an index which is incremented and mod'ed, acting like a ring buffer. The slaves_list itself is never modified and remains constant even if interrupted by an unexpected timeout. If a timeout fires during this process, we may end skipping a slave in the rotation, but that's a minor issue compared to the alternative.