Envek / sidekiq-fair_tenant

Sidekiq middleware to re-route “greedy” clients’ jobs to slower queues
MIT License
66 stars 2 forks source link

Error on integrating with sidekiq-scheduler #2

Open AlysonBasilio opened 1 week ago

AlysonBasilio commented 1 week ago

Hello folks,

Thank you for developing this gem. It provides such an elegant solution.

However, we've encountered an issue when integrating your gem with sidekiq-scheduler. Although we couldn't consistently reproduce the error, it seems to occur under certain conditions and results in the following stack trace:

TypeError: can't convert String into an exact number
-(/usr/local/bundle/ruby/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/core_ext/time/calculations.rb:314)
minus_with_duration(/usr/local/bundle/ruby/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/core_ext/time/calculations.rb:314)
minus_with_coercion(/usr/local/bundle/ruby/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/core_ext/time/calculations.rb:325)
block in assign_queue(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-fair_tenant-0.1.0/lib/sidekiq/fair_tenant/client_middleware.rb:59)
filter(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-fair_tenant-0.1.0/lib/sidekiq/fair_tenant/client_middleware.rb:57)
assign_queue(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-fair_tenant-0.1.0/lib/sidekiq/fair_tenant/client_middleware.rb:57)
block in call(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-fair_tenant-0.1.0/lib/sidekiq/fair_tenant/client_middleware.rb:22)
block (2 levels) in with(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:110)
handle_interrupt(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:109
block in with(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:109)
handle_interrupt(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:106)
with(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:106)
call(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-fair_tenant-0.1.0/lib/sidekiq/fair_tenant/client_middleware.rb:20)
traverse(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/middleware/chain.rb:182)
invoke(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/middleware/chain.rb:173)
push(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/client.rb:88)
block (2 levels) in enqueue_jobs(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/scheduled.rb:39)
each(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/scheduled.rb:33)
block in enqueue_jobs(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/scheduled.rb:33)
block in redis(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/config.rb:163)
block (2 levels) in with(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:110)
handle_interrupt(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:109)
block in with(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:109)
handle_interrupt(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:106)
with(/usr/local/bundle/ruby/3.3.0/gems/connection_pool-2.4.1/lib/connection_pool.rb:106)
redis(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/config.rb:160)
redis(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/component.rb:28)
enqueue_jobs(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/scheduled.rb:32)
enqueue(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/scheduled.rb:107)
block in start(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/scheduled.rb:99)
watchdog(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/component.rb:10)
block in safe_thread(/usr/local/bundle/ruby/3.3.0/gems/sidekiq-7.1.6/lib/sidekiq/component.rb:19)
block (2 levels) in thread_block_with_current_transaction(/usr/local/bundle/ruby/3.3.0/gems/newrelic_rpm-9.10.2/lib/new_relic/agent/tracer.rb:434)
capture_segment_error(/usr/local/bundle/ruby/3.3.0/gems/newrelic_rpm-9.10.2/lib/new_relic/agent/tracer.rb:357)
block in thread_block_with_current_transaction(/usr/local/bundle/ruby/3.3.0/gems/newrelic_rpm-9.10.2/lib/new_relic/agent/tracer.rb:433)

Our hypothesis is that some code is "stringifying" the sidekiq_options and converting the ActiveSupport::Duration objects in the per configuration into strings, causing the error.

We resolved the issue by using integer values instead of ActiveSupport::Duration objects. While this workaround is effective, it might be beneficial to include a warning in the README to enforce integer values usage, or implement a way of convert strings back to ActiveSupport::Duration objects if necessary.

Thank you for your attention to this matter. We appreciate your work on this gem.

Envek commented 4 days ago

Thanks for the report and kind words.

Sometimes, job configuration hash is being serialized and deserialized for whatever reason, and it is a bit of pain to understand why and when :face_with_head_bandage: (this gem already have some conversions to handle a bit different formats of it)

converting the ActiveSupport::Duration objects in the per configuration into strings

Do you have examples of such strings, what format they are in? Maybe it can be detected and parsed back to durations (e.g. if it is an ISO format)…