Scheduled jobs are not unlocked when deleted

[frsantos]

I think an extension is missing for Sidekiq::SortedEntry in sidekiq_unique_ext.rb file.

[mhenrixon]

Why? Sorted set inherits from Job and should be handled by that?

[frsantos]

Yes, but SortedEntry redefines delete

[mhenrixon]

It just calls parent.delete which is jobset and already taken care of. Didn't mean to close it just yet :)

[mhenrixon]

Have a failing test for it?

[frsantos]

SortedEntry#delete calls @parent.delete(score, jid), which is https://github.com/mperham/sidekiq/blob/master/lib/sidekiq/api.rb#L522, and that method does not call Job#delete

[frsantos]

No sorry, no test, I just stumbled on the problem.

[frsantos]

Sorry, no time for creating a proper test, but you can see it failing with this code:

class UniqueWorker
  include Sidekiq::Worker
  sidekiq_options unique: true

  def perform
    puts "Performing unique job"
  end
end

UniqueWorker.perform_in(1.hour)
=> "545161da8c5640bb088ac0f5"
Sidekiq::ScheduledSet.new.count
=> 1
Sidekiq.redis {|conn| conn.keys 'sidekiq_unique*' }
=> ["sidekiq_unique:274be02062ac878719e069e2b804e761"]
Sidekiq::ScheduledSet.new.each(&:delete)
=> nil
Sidekiq::ScheduledSet.new.count
=> 0
Sidekiq.redis {|conn| conn.keys 'sidekiq_unique*' } 
=> ["sidekiq_unique:274be02062ac878719e069e2b804e761"]
UniqueWorker.perform_in(1.hour)
=> nil
Sidekiq::ScheduledSet.new.count
=> 0

After delete, there should be no locks.

Updated with more test data

[pik]

@mhenrixon I can reproduce this. The notable part is that there are two types of delete defined in the sidekiq api one in Job and the other in JobSet - the later being the one which gets called when delete on a scheduled object runs; Although even if we fix that -- the delete defined under UniqueExtension would not be a sufficient replacement for the JobSet delete since the later can delete items by score.
Since the delete returns no information about which job was actually deleted it looks like SidekiqUnique needs to redeclare the entire method to hook into it. I don't know if there are any better ideas?

I have opened a tangential issue: sidekiq/sidekiq#2472

[frsantos]

I have now this hack in extensions to make scheduled jobs work. However, the scheduled polling has no way to hook other than redefining the whole method 😞

https://gist.github.com/frsantos/e7e765874eb396e1abb6