search

quirkey / resque-status

resque-status is an extension to the resque queue system that provides simple trackable jobs.
MIT License
515 stars 169 forks source link

Change Hash.statuses to use mget, reducing the number of redis queries #101

Closed tgsergeant closed 10 years ago

tgsergeant commented 10 years ago

The existing implementation makes n+1 queries to redis, this reduces it to two queries. Overall time complexity is unchanged, but overhead is significantly reduced.

quirkey commented 10 years ago

Thanks!

artm commented 10 years ago

For me this causes "Redis::CommandError at /statuses ... ERR wrong number of arguments for 'mget' command" (redis 2.6.13, redis gem 3.0.7, ruby 2.1, resque 1.25.1, resque-status 0.4.2)

The backtrace as reported by sinatra:

./vendor/bundle/gems/redis-3.0.7/lib/redis/client.rb in call
      raise reply if reply.is_a?(CommandError)
./vendor/bundle/gems/redis-3.0.7/lib/redis.rb in block in mget
      client.call([:mget] + keys, &blk)
./vendor/bundle/gems/redis-3.0.7/lib/redis.rb in block in synchronize
    mon_synchronize { yield(@client) }
/home/artm/.rbenv/versions/2.1.0/lib/ruby/2.1.0/monitor.rb in mon_synchronize
      yield
./vendor/bundle/gems/redis-3.0.7/lib/redis.rb in synchronize
    mon_synchronize { yield(@client) }
./vendor/bundle/gems/redis-3.0.7/lib/redis.rb in mget
    synchronize do |client|
./vendor/bundle/gems/redis-namespace-1.4.1/lib/redis/namespace.rb in method_missing
      result = @redis.send(command, *args, &block)
./vendor/bundle/gems/resque-status-0.4.2/lib/resque/plugins/status/hash.rb in mget
          vals = redis.mget(*status_keys)
./vendor/bundle/gems/resque-status-0.4.2/lib/resque/plugins/status/hash.rb in statuses
          mget(ids).compact
./vendor/bundle/gems/resque-status-0.4.2/lib/resque/status_server.rb in block in registered
        @statuses = Resque::Plugins::Status::Hash.statuses(@start, @end)
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in call
          proc { |a,p| unbound_method.bind(a).call }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block in compile!
          proc { |a,p| unbound_method.bind(a).call }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in []
            route_eval { block[*args] }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block (3 levels) in route!
            route_eval { block[*args] }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in route_eval
      throw :halt, yield
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block (2 levels) in route!
            route_eval { block[*args] }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block in process_route
        block ? block[self, values] : yield(self, values)
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in catch
      catch(:pass) do
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in process_route
      catch(:pass) do
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block in route!
          returned_pass_block = process_route(pattern, keys, conditions) do |*args|
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in each
        routes.each do |pattern, keys, conditions, block|
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in route!
        routes.each do |pattern, keys, conditions, block|
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block in dispatch!
        route!
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block in invoke
      res = catch(:halt) { yield }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in catch
      res = catch(:halt) { yield }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in invoke
      res = catch(:halt) { yield }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in dispatch!
      invoke do
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block in call!
      invoke { dispatch! }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block in invoke
      res = catch(:halt) { yield }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in catch
      res = catch(:halt) { yield }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in invoke
      res = catch(:halt) { yield }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in call!
      invoke { dispatch! }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in call
      dup.call!(env)
./vendor/bundle/gems/rack-protection-1.5.2/lib/rack/protection/xss_header.rb in call
        status, headers, body = @app.call(env)
./vendor/bundle/gems/rack-protection-1.5.2/lib/rack/protection/path_traversal.rb in call
        app.call env
./vendor/bundle/gems/rack-protection-1.5.2/lib/rack/protection/json_csrf.rb in call
        status, headers, body = app.call(env)
./vendor/bundle/gems/rack-protection-1.5.2/lib/rack/protection/base.rb in call
        result or app.call(env)
./vendor/bundle/gems/rack-protection-1.5.2/lib/rack/protection/base.rb in call
        result or app.call(env)
./vendor/bundle/gems/rack-protection-1.5.2/lib/rack/protection/frame_options.rb in call
        status, headers, body        = @app.call(env)
./vendor/bundle/gems/rack-1.5.2/lib/rack/nulllogger.rb in call
      @app.call(env)
./vendor/bundle/gems/rack-1.5.2/lib/rack/head.rb in call
    status, headers, body = @app.call(env)
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/show_exceptions.rb in call
      @app.call(env)
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in call
      result, callback = app.call(env), env['async.callback']
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in call
      @stack.call(env)
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in block in call
        synchronize { prototype.call(env) }
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in synchronize
          yield
./vendor/bundle/gems/sinatra-1.4.4/lib/sinatra/base.rb in call
        synchronize { prototype.call(env) }
./vendor/bundle/gems/rack-1.5.2/lib/rack/handler/webrick.rb in service
        status, headers, body = @app.call(env)
/home/artm/.rbenv/versions/2.1.0/lib/ruby/2.1.0/webrick/httpserver.rb in service
      si.service(req, res)
/home/artm/.rbenv/versions/2.1.0/lib/ruby/2.1.0/webrick/httpserver.rb in run
          server.service(req, res)
/home/artm/.rbenv/versions/2.1.0/lib/ruby/2.1.0/webrick/server.rb in block in start_thread
          block ? block.call(sock) : run(sock)
artm commented 10 years ago

The above is happening when I go to statuses in resque-web. Downgrading to resque-status 0.4.1 fixes the problem.

Hmm, could be my problem though. I don't see statuses from a running job that calls at. Hmm.

artm commented 10 years ago

Ah, sorry, that was me not paying attention. The above error happens if I create a job with status the traditional resque way (with Resque.enque). If I use ::create all is well with 0.4.2.

minusoneman commented 10 years ago

When running 

Resque::Plugins::Status::Hash.statuses

I get the same error as artm with 0.4.2 if my redis store is empty. If at least one status exists it returns the statuses as expected. 0.4.1 doesn't seem to be affected by this issue.

Ruby v2.0.0-p353, Redis Server v2.8.6

Sinatra backtrace

Redis::CommandError - ERR wrong number of arguments for 'mget' command:
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/redis-3.0.7/lib/redis/client.rb:97:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/redis-3.0.7/lib/redis.rb:802:in `block in mget'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/redis-3.0.7/lib/redis.rb:37:in `block in synchronize'
    /home/nicholai/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/monitor.rb:211:in `mon_synchronize'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/redis-3.0.7/lib/redis.rb:37:in `synchronize'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/redis-3.0.7/lib/redis.rb:801:in `mget'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/redis-namespace-1.4.1/lib/redis/namespace.rb:352:in `method_missing'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/resque-status-0.4.2/lib/resque/plugins/status/hash.rb:30:in `mget'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/resque-status-0.4.2/lib/resque/plugins/status/hash.rb:89:in `statuses'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/resque-status-0.4.2/lib/resque/status_server.rb:14:in `block in registered'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1593:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1593:in `block in compile!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:957:in `[]'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:957:in `block (3 levels) in route!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:976:in `route_eval'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:957:in `block (2 levels) in route!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:997:in `block in process_route'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:995:in `catch'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:995:in `process_route'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:955:in `block in route!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:954:in `each'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:954:in `route!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1067:in `block in dispatch!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1049:in `block in invoke'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1049:in `catch'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1049:in `invoke'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1064:in `dispatch!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:889:in `block in call!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1049:in `block in invoke'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1049:in `catch'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1049:in `invoke'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:889:in `call!'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:877:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-protection-1.5.2/lib/rack/protection/xss_header.rb:18:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-protection-1.5.2/lib/rack/protection/path_traversal.rb:16:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-protection-1.5.2/lib/rack/protection/json_csrf.rb:18:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-protection-1.5.2/lib/rack/protection/base.rb:50:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-protection-1.5.2/lib/rack/protection/base.rb:50:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-protection-1.5.2/lib/rack/protection/frame_options.rb:31:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-1.5.2/lib/rack/nulllogger.rb:9:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-1.5.2/lib/rack/head.rb:11:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/show_exceptions.rb:21:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:180:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:2004:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1469:in `block in call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1778:in `synchronize'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/sinatra-1.4.4/lib/sinatra/base.rb:1469:in `call'
    /home/nicholai/.rvm/gems/ruby-2.0.0-p353/gems/rack-1.5.2/lib/rack/handler/webrick.rb:60:in `service'
    /home/nicholai/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/webrick/httpserver.rb:138:in `service'
    /home/nicholai/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/webrick/httpserver.rb:94:in `run'
    /home/nicholai/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/webrick/server.rb:295:in `block in start_thread'
localhost.localdomain - - [05/Mar/2014:01:53:34 CST] "GET /statuses HTTP/1.1" 500 152156
http://localhost:5678/overview -> /statuses