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

V7 Beta 15 on_conflict: with Hash does not work on server #499

Closed
jasonbekolay opened this issue May 6, 2020 · 6 comments · Fixed by #503
Closed

V7 Beta 15 on_conflict: with Hash does not work on server #499

jasonbekolay opened this issue May 6, 2020 · 6 comments · Fixed by #503

Comments

@jasonbekolay
Copy link
Contributor

Describe the bug
Version 7 Beta 15 does not work on the server side only when given a Hash for on_conflict:. Server crashes when a conflict is detected.

Expected behavior
Conflict strategy is executed on the server when a conflict is detected.

Current behavior
Job crashes with stacktrace:

2020-05-06T17:25:16.817Z 67576 TID-owvovowmo WARN: NoMethodError: undefined method `to_sym' for {"client"=>"log", "server"=>"reschedule"}:Hash
Did you mean?  to_s
               to_set
2020-05-06T17:25:16.817Z 67576 TID-owvovowmo WARN: /Users/jasonbekolay/.rvm/gems/ruby-2.6.5@yapp/gems/sidekiq-unique-jobs-7.0.0.beta15/lib/sidekiq_unique_jobs/on_conflict.rb:34:in `find_strategy'
/Users/jasonbekolay/.rvm/gems/ruby-2.6.5@yapp/gems/sidekiq-unique-jobs-7.0.0.beta15/lib/sidekiq_unique_jobs/lock/base_lock.rb:148:in `server_strategy'
/Users/jasonbekolay/.rvm/gems/ruby-2.6.5@yapp/gems/sidekiq-unique-jobs-7.0.0.beta15/lib/sidekiq_unique_jobs/lock/while_executing.rb:40:in `block in execute'
/Users/jasonbekolay/.rvm/gems/ruby-2.6.5@yapp/gems/sidekiq-unique-jobs-7.0.0.beta15/lib/sidekiq_unique_jobs/logging.rb:103:in `block in with_logging_context'
/Users/jasonbekolay/.rvm/gems/ruby-2.6.5@yapp/gems/sidekiq-unique-jobs-7.0.0.beta15/lib/sidekiq_unique_jobs/logging.rb:118:in `block in with_configured_loggers_context'
/Users/jasonbekolay/.rvm/gems/ruby-2.6.5@yapp/gems/sidekiq-5.2.8/lib/sidekiq/logging.rb:48:in `with_context'

Worker class

class TestJob
  include Sidekiq::Worker

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

  def perform
    print "."
    sleep 1
    puts "."
  end
end

Additional context
I'm using Sidekiq 5.2.8. I believe the issue is that job hash is completed stringified on Sidekiq pulls it from Redis, and on_server_conflict is expecting symbols for keys in the hash.

@mhenrixon
Copy link
Owner

I recommend you upgrade to v7.0.16. It was fixed there

@jasonbekolay
Copy link
Contributor Author

Thank you for the quick response!

I don't see a release for 7.0.16. I tried against master and I'm still able to reproduce my issue. My reproduction steps are:

Worker class:

class TestJob
  include Sidekiq::Worker

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

  def perform(i)
    sleep 1
    puts "\n ----- #{i} -----\n"
  end
end

Start up multiple sidekiq servers. I have been running 4.

For my client, I boot up a rails console and run the following:

rand = Random.new
while true do
  i = rand.rand(100)
  puts "Queueing #{i}"
  3.times { Sidekiq::Client.enqueue(TestJob, i) }
  sleep 0.1
  3.times { Sidekiq::Client.enqueue(TestJob, i) }
  while Sidekiq::Queue.new.size > 0 do
    print "."
    sleep 1
  end
  puts "Done #{i}"
end

The client is solid, it works as expected when there's a conflict. If there's a conflict on the server side, it fails instead of rescheduling the job.

I think the issue is that on the server side, the job_hash gets serialized to JSON then deserialized, so the keys for the on_conflict hash are no longer symbols. This is what I'm trying to get it in #500

Thanks again for your help!

@mhenrixon
Copy link
Owner

Ok, I see you point, the solution is much simpler than I thought. Will try to create a new release today.

@jasonbekolay
Copy link
Contributor Author

Thanks! I've been running in production with #500 for the past few days and it has been working well. I'll switch to the latest release whenever you're able to create it.

mhenrixon added a commit that referenced this issue May 19, 2020
mhenrixon added a commit that referenced this issue May 19, 2020
* Fix access to both server and client conflict

Close #499
Close #500

* Fix documentation
@mhenrixon
Copy link
Owner

Released as v7.0.0.beta16

@jasonbekolay
Copy link
Contributor Author

Thank you for the release. Unfortunately the release broke client locking for me, as the on_conflict hash is not deeply stringified on the client side. Solution in #506 is working for me.

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

Successfully merging a pull request may close this issue.

2 participants