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

Unique Jobs Not Running with Version 7.1.26 #730

Closed
marclennox opened this issue Jul 29, 2022 · 12 comments · Fixed by #732
Closed

Unique Jobs Not Running with Version 7.1.26 #730

marclennox opened this issue Jul 29, 2022 · 12 comments · Fixed by #732
Assignees

Comments

@marclennox
Copy link
Contributor

I tried updating to 7.1.26 and now no unique jobs are actually running. Reverting to 7.1.25 resolves the issue.

@mhenrixon
Copy link
Owner

For me it is working fine:

worker | 2022-07-29T17:24:22.311Z pid=18000 tid=4s0 class=SlackNotifier jid=9ce8251c85717423e58aeb6d INFO: start
worker | 2022-07-29T17:24:22.326Z pid=18000 tid=4s0 class=SlackNotifier jid=9ce8251c85717423e58aeb6d elapsed=0.014 INFO: done

@mhenrixon
Copy link
Owner

Could you share some information about your setup @marclennox?

@marclennox
Copy link
Contributor Author

I'll provide more detail when I get back to this, but my jobs defined as follows are not running in the newer version, but fine in the previous.

  sidekiq_options lock: :until_and_while_executing,
                  on_conflict: {
                    client: :log,
                    server: :reject
                  }

@marclennox
Copy link
Contributor Author

They are all just going to the Dead queue

@mhenrixon
Copy link
Owner

Thanks for that, I'll try and replicate it in a local project!

@cgunther
Copy link

@marclennox any chance you're using redis-namespace?

I think the changes in v7.1.26 (namely #728) introduce a conflict with namespacing.

In v7.1.25, with no lock_timeout config set, when Locksmith#primed_async calls pop_queued, the wait was nil, so then pop_queued called rpoplpush.

In v7.1.26, pop_queued likely gets a timeout of 1 in the default scenario, thus triggering brpoplpush, which has a keyword argument, but I don't think redis-namespace properly handles it, which then leads to an error in the redis call, which seems to surface as the job conflicting with itself in this gem.

I didn't go terribly far down this rabbit hole, was only using namespacing in development, so ended up pulling it out completely, and that fixed things with v7.1.26. Jobs then ran fine in development and so far have run fine in production.

@mhenrixon
Copy link
Owner

Uff, redis-namespace strikes again... I can replicate the problem locally but fixing it I'm not even sure it is possible.

@mhenrixon
Copy link
Owner

This is the error message: Hash can't be coerced into Float

It seems like redis-namespace isn't compatible with newer redis versions. The redis-namespace uses the old version which took a float. Newer redis versions removed that argument:

def brpoplpush(source, destination, deprecated_timeout = 0, timeout: deprecated_timeout)

I don't want to use deprecated values if I can avoid it

@mhenrixon
Copy link
Owner

@cgunther @marclennox see if resque/redis-namespace#204 will be taken care of. There isn't that much I can do more than that.

@marclennox
Copy link
Contributor Author

@marclennox any chance you're using redis-namespace?

I think the changes in v7.1.26 (namely #728) introduce a conflict with namespacing.

In v7.1.25, with no lock_timeout config set, when Locksmith#primed_async calls pop_queued, the wait was nil, so then pop_queued called rpoplpush.

In v7.1.26, pop_queued likely gets a timeout of 1 in the default scenario, thus triggering brpoplpush, which has a keyword argument, but I don't think redis-namespace properly handles it, which then leads to an error in the redis call, which seems to surface as the job conflicting with itself in this gem.

I didn't go terribly far down this rabbit hole, was only using namespacing in development, so ended up pulling it out completely, and that fixed things with v7.1.26. Jobs then ran fine in development and so far have run fine in production.

Indeed I'm using redis namespace. So I should abandon this to get around the problem?

@mhenrixon
Copy link
Owner

Mike Perham recommends strongly against redis namespace so I have come to work around it.

I'll see if I can fix it but given the type of error; I wonder how many other hidden errors are because of keyword arguments and redis namespace.

@mhenrixon
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants