igorkasyanchuk / rails_performance

Monitor performance of you Rails applications (self-hosted and free)
https://www.railsjazz.com/
MIT License
957 stars 54 forks source link

Using with redis cluster #25

Closed strelok1111 closed 3 years ago

strelok1111 commented 3 years ago

I tried using this gem with redis cluster, and get error in this method

  def Utils.fetch_from_redis(query)
      #puts "\n\n   [REDIS QUERY]   -->   #{query}\n\n"

      keys   = RP.redis.keys(query)
      return [] if keys.blank?
      values = RP.redis.mget(keys)

      [keys, values]
    end

Redis::CommandError (CROSSSLOT Keys in request don't hash to the same slot)

then I change to this and all work

  def Utils.fetch_from_redis(query)
      #puts "\n\n   [REDIS QUERY]   -->   #{query}\n\n"

      keys   = RP.redis.keys(query)
      return [] if keys.blank?
      values = keys.collect { |k| RP.redis.get(k) }

      [keys, values]
    end
igorkasyanchuk commented 3 years ago

can this help https://aws.amazon.com/premiumsupport/knowledge-center/elasticache-crossslot-keys-error-redis/ ?

values = keys.collect { |k| RP.redis.get(k) } because it will make a X amount of requests to the redis and probably it's not well optimized

strelok1111 commented 3 years ago

I'm not familiar with redis, just want it to work.

haffla commented 3 years ago

I haven't tested this. But reading the article posted by @igorkasyanchuk I think a very simple solution would be to change https://github.com/igorkasyanchuk/rails_performance/blob/master/lib/rails_performance.rb#L26

from

@@redis = Redis::Namespace.new("#{::Rails.env}-rails-performance", redis: Redis.new)

to

@@redis = Redis::Namespace.new("{#{::Rails.env}-rails-performance}", redis: Redis.new)

This should guarantee that all rails_performance redis keys are going to end up in the same slot. @strelok1111 could you test this?

strelok1111 commented 3 years ago

Just test in production, changing config file make this gem work with redis cluster. @haffla thank you for simple solution. I think this case need to add in documentation.

igorkasyanchuk commented 3 years ago

@haffla if you can prepare a PR with fix or documentation it would be great. I'm closing this issue for now