mhenrixon/sidekiq-unique-jobs

8.0.1 Time on locks & changelog UI is incorrect/wrong

JeremiahChurch opened this issue · 5 comments

Describe the bug
took the 8.0.1/sidekiq 7 upgrade to try and resolve some non-releasing-lockd issues.

The locks screen is showing 1970-01-01 values in the 'since' column and the changelogs screen is showing 'bogus' for time. looks like a date integer coercion issue? Also appears to be creating empty lock values? I'm digging through the changes to see if I can find it...

the locks detail screen does show the correct time.

I cleared the locks and changeset (via the methods the UI calls) in case the upgrade from unique-jobs 7.x had problems - issue remains

image

image

image

a dump of a few of the values
SidekiqUniqueJobs::Digests.new.page(cursor:100, pattern: '*', page_size: 25)

[Lock status for prod_unique_jobs:3fdab3c234fd02f80a1da7f37d6dc9dc

          value: ffb803c6d0bf71fa81f5c10d
           info: {"worker"=>"DiscussionThreads::UpdateDiscussionsReadJob", "queue"=>"webhook_sync", "limit"=>nil, "timeout"=>0, "ttl"=>2109, "type"=>"until_executed", "lock_args"=>[17151205, 6], "time"=>1678370128.136117}
    queued_jids: []
    primed_jids: []
    locked_jids: ["ffb803c6d0bf71fa81f5c10d"]
     changelogs: []
,
  Lock status for 1678370128.1982968

          value: 
           info: 
    queued_jids: []
    primed_jids: []
    locked_jids: []
     changelogs: []
,
  Lock status for prod_unique_jobs:e46aff0823ab33ec7e0208af451188a7

          value: b936b6065e0431af2c9fa059
           info: {"worker"=>"DiscussionThreads::UpdateDiscussionsReadJob", "queue"=>"webhook_sync", "limit"=>nil, "timeout"=>0, "ttl"=>1922, "type"=>"until_executed", "lock_args"=>[12455593, 3], "time"=>1678370352.083641}
    queued_jids: []
    primed_jids: []
    locked_jids: ["b936b6065e0431af2c9fa059"]
     changelogs: []
,
  Lock status for 1678370352.1326859

          value: 
           info: 
    queued_jids: []
    primed_jids: []
    locked_jids: []
     changelogs: []
,
  Lock status for prod_unique_jobs:41b36112361e536032784f2c9096655f

          value: 99fd4a3c921e69e7f38235d0
           info: {"worker"=>"DiscussionThreads::UpdateDiscussionsReadJob", "queue"=>"webhook_sync", "limit"=>nil, "timeout"=>0, "ttl"=>2169, "type"=>"until_executed", "lock_args"=>[17856623, 7], "time"=>1678370243.577242}
    queued_jids: []
    primed_jids: []
    locked_jids: ["99fd4a3c921e69e7f38235d0"]

Expected behavior
correct dates

Current behavior
incorrect dates and extra locks?

Additional context
sidekiq 7.0.6
sidekiq-unique-jobs 8.0.1
sidekiq-scheduler 5.0.2

It looks like the result of digests[1].map in Digests#page has changed because the underlying values are stored/returned differently and that is causing the multiple rows to be returned - the 'blank' is just the time for the lock

    def page(cursor: 0, pattern: SCAN_PATTERN, page_size: 100)
      redis do |conn|
        total_size, digests = conn.multi do |pipeline|
          pipeline.zcard(key)
          pipeline.zscan(key, cursor, match: pattern, count: page_size)
        end
        puts "\n\ndigests: #{digests[1].inspect}\n\n"
        # NOTE: When debugging, check the last item in the returned array.
        [
          total_size.to_i,
          digests[0].to_i, # next_cursor
          digests[1].map { |digest, score| Lock.new(digest, time: score) }, # entries
        ]
      end

returns

digests: ["0", 
["prod_unique_jobs:2e3f377528bd7e5dafca80f993144d81", "1678393091.0208826", 
"prod_unique_jobs:94f085827423f76b1f3e610bf316ad1c", "1678393399.1548831"]
]

when instead it should be nested more like:

digests: ["0", 
[
["prod_unique_jobs:2e3f377528bd7e5dafca80f993144d81", "1678393091.0208826"], 
["prod_unique_jobs:94f085827423f76b1f3e610bf316ad1c", "1678393399.1548831"]
]
]

a monkey patch to use activesupport's in_groups_of(2) gets the page looking good without my changes in #762

module SidekiqUniqueJobs
  class Digests
    def page(cursor: 0, pattern: SCAN_PATTERN, page_size: 100)
      redis do |conn|
        total_size, digests = conn.multi do |pipeline|
          pipeline.zcard(key)
          pipeline.zscan(key, cursor, match: pattern, count: page_size)
        end

        # NOTE: When debugging, check the last item in the returned array.
        [
          total_size.to_i,
          digests[0].to_i, # next_cursor
          digests[1].in_groups_of(2).map { |digest, score| Lock.new(digest, time: score) }, # entries
        ]
      end
    end
  end
end

Can I close this then @JeremiahChurch?

Can I close this then @JeremiahChurch?

@mhenrixon without that monkey patch the locks page is still broken. I'm assuming it should generally be fixed?

As-is on 8.0.2 without that patch you'll see

image

with the monkey patch the 'extra' lines are gone

image

If you're alright with active support as a depend I'll pull a PR together if you'd like?

@JeremiahChurch I don't think the dependency on active_support is needed. each_slice should do the job just as well.

@JeremiahChurch I don't think the dependency on active_support is needed. each_slice should do the job just as well.

Great suggestion, I'll have a look at that.