Skip to content
This repository has been archived by the owner on Apr 1, 2023. It is now read-only.

Doesn't work if multiple services share the same redis #135

Closed
albertosaurus opened this issue Jun 27, 2018 · 2 comments
Closed

Doesn't work if multiple services share the same redis #135

albertosaurus opened this issue Jun 27, 2018 · 2 comments

Comments

@albertosaurus
Copy link

From this page: https://github.com/mperham/sidekiq/wiki/Middleware

Obscure Scenario: If you share a single redis instance between multiple services, and use client-side middleware on your Sidekiq server, be warned that it might have to deal with workers from unexpected queues. Sidekiq uses a singe redis set to handle scheduled workers, and any server might requeue a worker that's due to be processed, even if it's not for a queue it was configured to process. When it does so, your middleware might be passed a worker name that does not exist in the current ruby context and you might see uninitialized constant exceptions or similar errors.

Since version 0.8.0 the middleware client is doing a Module.const_get without handling any potential NameError. This causes annoying breakage. I'd like to propose something like this:

  def call(worker_class, msg, queue, redis_pool=nil)

    # Determine the actual job class
    klass = msg["args"][0]["job_class"] || worker_class rescue worker_class

    job_class = if klass.is_a?(Class)
                  klass
                elsif Module.const_defined?(klass) # Ensure constant is defined before attempting access 
                  Module.const_get(klass)
                else
                  nil
                end

    # Store data if the job is a Sidekiq::Status::Worker
    if job_class && job_class.ancestors.include?(Sidekiq::Status::Worker)
      initial_metadata = {
        jid: msg['jid'],
        status: :queued,
        worker: Sidekiq::Job.new(msg, queue).display_class,
        args: display_args(msg, queue)
      }
      store_for_id msg['jid'], initial_metadata, job_class.new.expiration || @expiration, redis_pool
    end

    yield

  end

end
@baburdick
Copy link

+1

@kenaniah
Copy link
Collaborator

Fixed in v1.1.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants