add ability to configure global Redis namespace prefix

[dammer]

#133

SCAN_UUID=ryhNEB9tD2K8pvZDdNZKnT ./bin/app

Mosquito.configure do |settings|
  settings.redis_url = ENV["REDIS_URL"]? || "redis://localhost:6379/5"
  settings.global_prefix = ENV["SCAN_UUID"]?
end

Redis:

1715597220.661123 [5 127.0.0.1:38410] "keys" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:scheduled:*"
1715597220.962453 [5 127.0.0.1:38410] "keys" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:waiting:*"
1715597220.962801 [5 127.0.0.1:38410] "keys" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:scheduled:*"
1715597221.263722 [5 127.0.0.1:38410] "keys" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:waiting:*"
1715597221.263946 [5 127.0.0.1:38410] "keys" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:scheduled:*"
1715597221.565036 [5 127.0.0.1:38410] "keys" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:waiting:*"
1715597221.565360 [5 127.0.0.1:38410] "keys" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:scheduled:*"

[robacarp]

Wonderfully done @dammer. Do you think there's a need for both KEY_PREFIX and global_prefix? I can't decide 🤔

If not, it might suffice to simply change what KEY_PREFIX itself means, or how it is sourced:

   # KEY_PREFIX becomes a getter or class getter:
   getter key_prefix : String { Mosquito.configuration.global_prefix }

   # Or just ditch key_prefix altogether and read from configuration:
   def self.build_key(*parts)
      KeyBuilder.build Mosquito.configuration.global_prefix, *parts
   end

   # and in Configuration:
   property key_prefix : String = "mosquito"

[dammer]

Thanks, Rob. I made the decision to keep the "mosquito" prefix based on the fact that without it, the cleanup script would affect the entire global key space, and also because not all code references the constant and sometimes uses a direct string value

mosquito/src/mosquito/redis_backend.cr

Line 133 in 703a90a

runner_prefix = "mosquito:runners:"

Additionally, this may allow us to maintain maximum backward compatibility with third parties.
The first option looks like the best solution to me, but since the getter will be cached, won't there be a risk of calling it before the configuration is applied? In this case, we'll need to reset the cached value when we change the global_prefix option in the configuration.

[dammer]

I simplified my solution a bit by adding Nil handling in the KeyBuilder, which allows removing the condition in build_key and simplifies the solution slightly. I also removed the direct use of "mosquito" without key_build.

[dammer]

UPD: I noticed an inconsistency when running the PR on the master branch: the periodic job key is not prefixed. Take a look at "mosquito:periodic_jobs:StatusJob", all other keys are okay. Could it be that the Coordinator runs before the configuration?

127.0.0.1:6379[5]> MONITOR
OK
1715696179.556765 [5 127.0.0.1:34060] "select" "5"
1715696179.556848 [5 127.0.0.1:34062] "select" "5"
1715696179.556977 [5 127.0.0.1:34060] "script" "load" "  if redis.call(\"get\",KEYS[1]) == ARGV[1] then\n      return redis.call(\"del\",KEYS[1])\n  else\n      return 0\n  end"
1715696179.557012 [5 127.0.0.1:34062] "script" "load" "  if redis.call(\"get\",KEYS[1]) == ARGV[1] then\n      return redis.call(\"del\",KEYS[1])\n  else\n      return 0\n  end"
1715696179.557088 [5 127.0.0.1:34060] "set" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:coordinator:football" "0e494ddf5ac9522d" "ex" "10" "nx"
1715696179.557137 [5 127.0.0.1:34062] "zrange" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:queues" "0" "-1"
1715696179.557374 [5 127.0.0.1:34076] "select" "5"
1715696179.557636 [5 127.0.0.1:34076] "hget" "mosquito:periodic_jobs:StatusJob" "last_executed_at"
1715696179.557880 [5 127.0.0.1:34062] "hset" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:job_run:1715696179557:8" "enqueue_time" "1715696179557" "type" "status_job" "retry_count" "0"
1715696179.557981 [5 127.0.0.1:34076] "lpush" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:waiting:status_job" "1715696179557:8"
1715696179.558000 [5 127.0.0.1:34076] "zadd" "ryhNEB9tD2K8pvZDdNZKnT:mosquito:queues" "1715696179" "status_job"
1715696179.558063 [5 127.0.0.1:34062] "hset" "mosquito:periodic_jobs:StatusJob" "last_executed_at" "1715696179"

[dammer]

Yep it is early than configuration

mosquito/src/mosquito/periodic_job.cr

Line 24 in 703a90a

Mosquito::Base.register_job_interval \{{ @type.id }}, \{{ interval }}

fixed e315789

[robacarp]

@dammer nice improvements, and well thought out too.

[robacarp]

Normally, I'd ask for ameba fixes to be submitted separately, just so the changes history doesn't get too muddled. In this case, 👍 and thank you.

[robacarp]

Nice improvement. The use of #keys here is still a performance implication and can abuse redis servers regardless of the search string, but that's an unrelated issue.

[dammer]

Perhaps it's possible to use build_key once here, and then cache the value in the getter runner_prefix to improve performance, but I'm not sure.

[robacarp]

I believe that this test failure on 1.4 is unrelated to this code -- and that 1.4 should probably not be a target anymore.