Delayed Sidekiq strategy errors by passing invalid Array type to redis client

[arinhouck]

I am trying to import using the Delayed sidekiq strategy. However I am getting a redis client error for passing an invalid array type. From readings on other repository issues this seems to be a common case with gems integrated with redis commands which pass nested arrays to sadd call.

---

Expected behavior

I would expect it to kick off a redis job for sidekiq but it errors.

Actual behavior

The stacktrace throws an error from redis-client. See here someone else had come across this similar error for sidekiq-batch gem that had to be fixed to pass the correct format.

Stacktrace

TypeError: Unsupported command argument type: Array
from /Users/myuser/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/redis-client-0.22.2/lib/redis_client/command_builder.rb:39:in `block in generate'

This issue seemed to bubble up to this LUA script call here.

Steps to reproduce the problem

I setup with the standard delayed sidekiq settings

Chewy.settings = {
  strategy_config: {
    delayed_sidekiq: {
      latency: 3,
      margin: 2,
      ttl: 60 * 60 * 24
    }
  }
}

You can reproduce by:

ProductsIndex.import!([1], strategy: :delayed_sidekiq)

Or even more specific I was diagnosing the call further down the stack to validate the inputs were correct.

Chewy::Strategy::DelayedSidekiq::Scheduler.new(ProductsIndex, [1]).postpone

This is only applicable on 8.0.0.pre.beta tag.

Version Information

Share here essential version information such as:

• Chewy version: 8.0.0.pre.beta (commit = 6a2176f)
• Elasticsearch version: 8.5.1
• Ruby version: 3.1.2
• Rails version: 6.1.7.9
• Redis gem version: 4.2.0, 4.8.1, and 5.0.2 (latest)

[arinhouck]

I have done further debugging and found the problematic input:

On this line

The following input from the variable element is an array:

["chewy:delayed_sidekiq:ProductsIndex:1730932392", "chewy:delayed_sidekiq:ProductsIndex:timechunks"]

It's a bit tricky to debug because of the static LUA script.

If I debug one level up the keys argument with the value [timechunk_key, timechunks_key] from this line matched the same value:

["chewy:delayed_sidekiq:ProductsIndex:1730932851", "chewy:delayed_sidekiq:ProductsIndex:timechunks"]

So somewhere this keys variable is passed along to one of those redis calls in the LUA script as an array.

I'm thinking it's the sadd or zrank call but I'm not as familiar with LUA.

Also this LUA script was failing too for a similar error:
784fe38#diff-3dd6f3f828085be7751a3490204c6604a00727b81650637c632608d22ee28138R42

[arinhouck]

Okay it came down to this PR that caused this error for delayed_sidekiq. If I revert this specific change everything is functional again.
#937

cc @skcc321 @konalegi

My guess is the the LUA script is evaluating something as an array instead of a single string from one of the local variables. Something is different from LUA vs ruby in the way the inputs are transformed.

[arinhouck]

Well this is still broken, based on upvotes it seems like this is an issue for others too. @skcc321 or any others can you verify this error is reproducible? If not reproducible, can you provide which version of sidekiq you may be using? As I can't find a way to make delayed sidekiq functional with the LUA script change at all.

After further digging, I discovered the LUA script is not being formatted correctly by the Sidekiq redis client wrapper (instance of Sidekiq::RedisClientAdapter::CompatClient must behave differently and the specs mock Sidekiq redis call). It is sending nested arrays which is problematic as you can see from the link above to redis-client error.

I found this PR that showed a LUA script being called within Sidekiq. I mimicked a similar convention and the proper data structure was sent for keys and argv. Now everything works with the LUA script without issue.

For example I did something like this for both worker.rb and scheduled.rb LUA script:

        members = extract_members(redis, keys: [], argv: [processor_key, score, Scheduler::KEY_PREFIX])
....
    def extract_members(conn, keys: [], argv: [])
      if @lua_extract_members_sha.nil?
        @lua_extract_members_sha = conn.script(:load, LUA_SCRIPT)
      end

      conn.call("EVALSHA", @lua_extract_members_sha, keys.size, *keys, *argv)
    rescue RedisClient::CommandError => e
      raise unless e.message.start_with?("NOSCRIPT")

      @lua_extract_members_sha = nil
      retry
    end

I believe this is our fix, but still don't fully understand how the original change was tested and with what version of sidekiq.

[tylerwillingham]

Can confirm, I'm experiencing this issue on Chewy 7.6.0 and Sidekiq 7.3.9