until_expired is not setting TTL

[axos88]

Describe the bug
As far as I understand, this should create a lock with a TTL of 5 seconds, and allow the job to be enqueued every 5 seconds only. I'm not sure wether the counter should start from the time on enqueument, time it stated executing, or the time it finished. Anyways the job is pretty fast, so these are almost the same, but some documentation would be helpful.

Expected behavior

Allow the job to be enqueued later.

Current behavior
It gets permanently locked.
Redis shows the TTL of the key with a value of -1

Airbnb::NtakSubmit.perform_async(2443067770)
 => "6aa1b02d0b5d9b799ddbf2dc" 
2.7.0 :014 > MyWorker.perform_async(2443067770) # 1 second later, expected to be nil
 => nil 
2.7.0 :015 > MyWorker.perform_async(2443067770) # 6 seconds later it should allow it
 => nil 
2.7.0 :016 > MyWorker.perform_async(2443067770) # 1 hour later still nothing

Not sure what Grabbed and EXISTS are, but both have permanently been added:

127.0.0.1:6379> ttl uniquejobs:8f0da2dd94bb1b8798a5632890d6f9c6:GRABBED
(integer) -1
127.0.0.1:6379> ttl uniquejobs:8f0da2dd94bb1b8798a5632890d6f9c6:EXISTS
(integer) -1

Worker class

  class MyWorker
    include Sidekiq::Worker
    sidekiq_options retry: 0, lock: :until_expired, lock_ttl: 5
  end

[mhenrixon]

Hi @axos88, thanks for the report. May I ask which versions you are on? Sidekiq, sidekiq unique jobs, redis ruby client, redis server etc?

Also would help if you could shed a light on any configuration you have for sidekiq like what other middleware are you using and so on and so forth?

[axos88]

Hah. Yeah, sorry :)

Ruby 2.7.0

    redis (4.1.3)
    sidekiq (6.0.4)
    sidekiq-unique-jobs (6.0.18)
    sidekiq-status (1.0.2)
    sidekiq-cron (1.1.0)
    sidekiq-enqueuer (2.1.0)

No other middleware than the ones listed above.

[blanchma]

Similar issue

[mhenrixon]

@axos88 can you replicate this in a test? I tried the following but it seems to be working in the test suite at least:

RSpec.describe "Issue 468" do
  class MyWorker
    include Sidekiq::Worker
    sidekiq_options retry: 0, lock: :until_expired, lock_ttl: 5
  end

  specify do
    expect(MyWorker.perform_async).not_to eq(nil)
    expect(MyWorker.perform_async).to eq(nil)
    sleep(6)
    expect(MyWorker.perform_async).not_to eq(nil)
  end
end

This was tested on v6.0.19, I yanked .15-.18 because of a bug that was introduced in .15.

[mhenrixon]

The only thing I can think of would be that one of the other gems are incompatible or misconfigured (in regards to how they relate to this gem).

[mhenrixon]

Sorry, I tested it on the wrong branch! Was a little late last night 🤔I can replicate it with this spec on branch v6.x. I cannot replicate the issue on v7.0.0.beta11

[mhenrixon]

@axos88 it seems like you are using v7 configuration for v6. lock_ttl was introduced in v7 and the right key to use is lock_expiration. That said, it is a small thing to allow lock_ttl in the v6 branch so I added it in #479.

It will be released as v6.0.20 in 20 minutes or so.