Update to redis-rb 3.1.0; replace #unknown_items transaction with a script

Gemfile

@@ -5,8 +5,8 @@ gem "json"
 gem "activesupport"
 gem "i18n"
 gem "erubis"
-gem "hiredis", "~> 0.3.1"
-gem "redis", "~> 2.2.0", :require => ["redis/connection/hiredis", "redis"]
+gem "hiredis"
+gem "redis", "~> 3.0", :require => ["redis/connection/hiredis", "redis"]
 gem "bcrypt-ruby"
 
 group :development do

Gemfile.lock

@@ -25,7 +25,7 @@ GEM
     rack-test (0.6.1)
       rack (>= 1.0)
     rake (0.9.2.2)
-    redis (2.2.2)
+    redis (3.1.0)
     rspec (2.10.0)
       rspec-core (~> 2.10.0)
       rspec-expectations (~> 2.10.0)
@@ -50,12 +50,12 @@ DEPENDENCIES
   bcrypt-ruby
   cucumber
   erubis
-  hiredis (~> 0.3.1)
+  hiredis
   i18n
   json
   rack-test
   rake
-  redis (~> 2.2.0)
+  redis (~> 3.0)
   rspec
   shotgun
   sinatra

models/tracker/statistics.rb

@@ -72,7 +72,8 @@ def claims_per_downloader(regexp=nil)
         claims = redis.hgetall("#{ prefix }claims")
         out = redis.zrange("#{ prefix }out", 0, -1, :with_scores=>true)
         claims_per_downloader = ActiveSupport::OrderedHash.new{ |h,k| h[k] = [] }
-        out.each_slice(2) do |item, time|
+
+        out.each do |item, time|
           if claims[item]
             ip, downloader = claims[item].split(" ", 2)
           else
@@ -230,9 +231,9 @@ def stats
           redis.zcard("#{ prefix }out")                  # 6
         end
 
-        domain_bytes = Hash[*resp[0]]
-        downloader_bytes = Hash[*resp[1]]
-        downloader_count = Hash[*resp[2]]
+        domain_bytes = resp[0]
+        downloader_bytes = resp[1]
+        downloader_count = resp[2]
         total_items_done = resp[3].to_i
         total_items_todo = resp[4].to_i + resp[5].to_i
         total_items_out = resp[6].to_i

models/tracker/transactions.rb

@@ -66,7 +66,7 @@ def blocked?(keys)
           keys.each do |key|
             redis.sismember("#{ prefix }blocked", key)
           end
-        end.any?{|r|r.to_i==1}
+        end.any?
       end
 
       def blocked
@@ -215,11 +215,11 @@ def item_status(item)
           redis.hexists("#{ prefix }claims", item)
           redis.sismember("#{ prefix }done", item)
         end
-        if replies[0] == 1 or replies[1] == 1
+        if replies[0] or replies[1]
           :todo
-        elsif replies[2] == 1
+        elsif replies[2]
           :out
-        elsif replies[3] == 1
+        elsif replies[3]
           :done
         else
           nil
@@ -249,21 +249,28 @@ def item_claimant(item)
       end
 
       def unknown_items(items)
-        replies = redis.pipelined do
-          items.each do |item|
-            redis.sismember("#{ prefix }todo", item)
-            redis.sismember("#{ prefix }todo:secondary", item)
-            redis.hexists("#{ prefix }claims", item)
-            redis.sismember("#{ prefix }done", item)
+        begin
+          script_ident = redis.script(:load, ITEM_UNKNOWN_SCRIPT)
+
+          replies = redis.pipelined do
+            items.each { |item| redis.evalsha(script_ident, [], [prefix, item]) }
+          end
+        rescue Redis::CommandError => e
+          if e.message =~ /NOSCRIPT/
+            retry
+          else
+            raise e
           end
         end
 
         to_add = []
-        replies.each_slice(4).each_with_index do |response, idx|
-          if response==[0,0,0,0]
+
+        replies.each_with_index.each do |unknown, idx|
+          if unknown
             to_add << items[idx]
           end
         end
+
         to_add
       end
 
@@ -285,13 +292,15 @@ def add_items_to_queue!(queue, items)
 
         added = []
         queue_key = "#{ prefix }#{ queue }"
+
         replies = redis.pipelined do
           items.each do |item|
             redis.sadd(queue_key, item)
           end
         end
+
         replies.each_with_index do |reply, idx|
-          added << items[idx] if reply==1
+          added << items[idx] if reply
         end
         added
       end
@@ -456,6 +465,26 @@ def mark_item_done(downloader, item, bytes, done_hash)
 
       private
 
+      ##
+      # Used by #unknown_items to determine whether an item is already in the
+      # todo, done, or claimed sets.
+      ITEM_UNKNOWN_SCRIPT = %Q{
+        local item = ARGV[2]
+        local sets = {ARGV[1]..'todo', ARGV[1]..'secondary', ARGV[1]..'done'}
+
+        for index, set_name in ipairs(sets) do
+          if redis.call('sismember', set_name, item) ~= 0 then
+            return false
+          end
+        end
+
+        if redis.call('hexists', ARGV[1]..'claims', item) ~= 0 then
+          return false
+        end
+
+        return true
+      }
+
       # After an item has been marked done, this function should be called
       # to update the statistics.
       def update_stats_when_done(downloader, bytes)
@@ -505,15 +534,18 @@ def update_stats_when_done(downloader, bytes)
               redis.call('APPEND', KEYS[4], entry)
               redis.call('HSET', KEYS[1], 'total bytes', ARGV[2])
             end
-          }, 4,
-          "#{ prefix }chart:previous_timestamp",
-          "#{ prefix }chart:downloader_bytes:#{ downloader }",
-          "#{ prefix }chart:total_items",
-          "#{ prefix }chart:total_bytes",
-          downloader, minute,
-          "%013x" % downloader_bytes,
-          "%08x" % done_count,
-          "%013x" % total_bytes)
+          },
+          ["#{ prefix }chart:previous_timestamp",
+           "#{ prefix }chart:downloader_bytes:#{ downloader }",
+           "#{ prefix }chart:total_items",
+           "#{ prefix }chart:total_bytes"
+          ],
+          [downloader,
+           minute,
+           "%013x" % downloader_bytes,
+           "%08x" % done_count,
+           "%013x" % total_bytes
+          ])
       end
     end
   end

spec/tracker_spec.rb

@@ -27,7 +27,7 @@ module UniversalTracker
       @tracker.ip_blocked?("192.0.0.2").should == true
 
       @tracker.ip_block_log.should == ['{"test":123}']
-      @tracker.blocked?(["192.0.0.1"])
+      @tracker.blocked?(["192.0.0.1"]).should be_true
     end
 
     it "should block downloaders" do