Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior with until_executed #250

Closed
mwpastore opened this issue Oct 26, 2017 · 6 comments
Closed

Unexpected behavior with until_executed #250

mwpastore opened this issue Oct 26, 2017 · 6 comments

Comments

@mwpastore
Copy link

I have some long-running (infinite loop) jobs that are :unique=>:until_executed. The README says, "By default the lock doesn't expire at all," and it specifically calls out only :while_executing and :until_and_while_executing (i.e. not :until_executed) as locking modes. However, when I start this job, I notice that its uniquejobs:{digest} key takes a TTL of 1800. After 1,800 seconds, if a process attempts to queue one of these jobs (while the original instance is still running), it will launch a duplicate job. I tried adding :lock_expiration=>nil to address this issue, but the behavior persists.

  • Am I using the correct mode for this type of job? Is there a more appropriate type?
  • Does this mode take the default_queue_lock_expiration by design, or is that a bug (i.e. it should be nil)?
  • Is there another way to force a non-expiring uniquejobs:{digest} key, or is that a bug (i.e. it should be settable to nil)?
@ragesoss
Copy link

This sounds like the same thing I ran into today. @mwpastore, were you able to find a workaround?

@mwpastore
Copy link
Author

Unfortunately, not really. I switched to using a watchdog thread for these long-running jobs that manually looks to see if the job is running, queued, or pending retry, and starts it if not.

# frozen_string_literal: true
require_relative 'worker_classes'

WATCHED_WORKER_CLASSES = [WatchMailboxWorker, WatchSubsWorker].freeze

default = Sidekiq::Queue.new
workers = Sidekiq::Workers.new
retry_set = Sidekiq::RetrySet.new

Thread.new do
  sleep 60

  WATCHED_WORKER_CLASSES.each do |worker_class|
    # Is job queued?
    next if default.map(&:klass)
      .include?(worker_class.name)

    # Is job running?
    next if workers.map { |*, work| work['payload']['class'] }
      .include?(worker_class.name)

    # Is job pending an automatic retry?
    next if retry_set.map(&:klass)
      .include?(worker_class.name)

    worker_class.perform_async
  end

  redo
end

@ragesoss
Copy link

The unique_expiration option will override the default (although I don't think setting nil will work). Here's what I did: WikiEducationFoundation/WikiEduDashboard@04a67da

@ragesoss
Copy link

I agree that the 30 minute timeout is unexpected, and it was hard to discover. Defaulting to never expiring would be better, in my opinion.

@mhenrixon
Copy link
Owner

mhenrixon commented Jun 9, 2018

I will take care of this for the next major version and add a note about this in the readme's.

@mhenrixon
Copy link
Owner

Version 6 won't have this problem and it will take care of the upgrade automatically. lock_expiration should really not be used for anything at all since the keys WILL be removed. Maybe I should remove that option for all jobs except UntilExpired

Keep an eye out on the README and CHANGELOG for more information about the changes.

@mhenrixon mhenrixon added this to the Version 6.0 milestone Jun 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants