Sidekiq 4 compatibility

[albertodega]

Hi, i've used this wonderful gems for months without any kind of problem with background jobs processed by Sidekiq 3. I've just updated to Sidekiq 4, which is now fully thread based, removing the dependency with Celluloid as you can state in the creator blog here: http://www.mikeperham.com/2015/10/14/optimizing-sidekiq/ Actually, request_store on Sidekiq 4 has started to work randomly. On sidekiq 3 i used to set some variables in request store at the very beginning of the worker process. those variables where used by activerecord to set some default scopes inside the process. at the end of the worker execution the thread was supposed to die, with the setted up variables, so any worker launched after the last one started with a clean status. My opinion is that the new full threaded system of sidekiq makes variables used with request store available to other workers launched in the same moment or next to the last one. According to the new sidekiq architecture, is it possible that the behavior of request_store is influenced in that sense? Is it possible to find a solution for this? Thank you in advance for your time, fully available if you need other infos or internals.

[steveklabnik]

I am not sure how Sidekiq and request_store would really work together: request_store is for incoming HTTP requests to a web server, but Sidekiq is for background jobs?

[albertodega]

exactly. sidekiq 3 works with celluloid: it forks a child process for every work. in this fork i set a variable with your gem, which is then cleaned up when the process is finished and the fork is exited. everything works smoothly as expected. sidekiq 4 now works with threads instead. i actually don't understand how and if it uses shared portions of memory or variables doing so, but it seems that concurrent jobs shares the same Thread.current process. i've filed up an issue in sidekiq project to try to understand why it's not working anymore as expected, you can find the discussion here: https://github.com/mperham/sidekiq/issues/2727 thank you again!

[pomartel]

@albertodega Did you find a clean way to clear the request_store state after each job with sidekiq 4? I also get the issue that my thread values are not cleared and are available to the next job. This is only logical since request_store was designed with http requests in mind and not background jobs. But I would love a way to use it for both.

[pomartel]

I think RequestStore.clear! could be called after each Sidekiq job. This can be done through a Sidekiq server side middleware. I haven't yet tested it but I will give it a go after Christmas: https://github.com/mperham/sidekiq/wiki/Middleware

[albertodega]

@pomartel i found i was having an old, still wirking process of sidekiq 3 running with the updated sidekiq 4 instance. it seems that douring the upgrade something went wrong with the killing of the older process, which maybe was still serving some requests. I've discovered it 2 days ago, anc since then there is only sidekiq 4 running, and everything seems fine. I've however added a forcing setter at the very beginning of every worker, which takes care of setting or resetting the request store variable according to what i need inside of the worker. you can find an example of what i'm talking abount in my second post of the referenced issue on sidekiq's project. I'll let you know if something changes

[pomartel]

This should be the cleanest way to do the job right :

class RequestStoreMiddleware
  def call(worker, msg, queue)
    yield
  ensure
    ::RequestStore.clear! if defined?(::RequestStore)
  end
end

Sidekiq.configure_server do |config|
  config.server_middleware do |chain|
    chain.add RequestStoreMiddleware
  end
end

[mperham]

@albertodega I think you are confused. Sidekiq has never used child processes to process anything; it has always used threads. Sidekiq 3 used Celluloid to manage the threads, Sidekiq 4 manages the threads itself.

@pomartel That's exactly right. Server-side middleware should clear the state in an ensure block.

[ibrahima]

This seems to not be an actual issue with Sidekiq compatibility, but perhaps the middleware solution should be documented someplace? Seems like this issue should be closed in any case. For a second when I was checking this gem out to see if it would satisfy my use case the presence of this issue scared me, but reading through the issue I understood that it's not a problem per se, just an unintended use case that needs to be handled specifically.

While this gem doesn't really interact with Sidekiq directly, if your code uses RequestStore someplace that's shared across Rails and Sidekiq you probably don't want a separate code path just to store the data in a different way inside Sidekiq workers.

[macfanatic]

Just to share with the community, I created this gem to provide that Sidekiq middleware, complete with a Railtie so it's a dropin for Rails projects.

https://rubygems.org/gems/request_store-sidekiq

I'll submit a PR as well to add some documentation to the README on this project.

[steveklabnik]

I've merged the PR to add this to the README, so let's give this a close!