Possible invalid start and end values sent to Redis for ZRANGE #213
Comments
|
FWIW, I currently use this workaround (monkey patch): # Monkey patch two private methods in Stoplight::DataStore::Redis such that
# Stoplight does not end up sending the strings "Infinity" and "-Infinity" as
# start/min and stop/max values in Redis ZREMRANGEBYSCORE and ZRANGE commands.
# See:
# * https://github.com/bolshakov/stoplight/issues/213
# * https://redis.io/commands/zremrangebyscore/
# * https://redis.io/commands/zrange/
module Stoplight
module DataStore
class Redis
private
def remove_outdated_failures(light, time, transaction: @redis)
failures_key = failures_key(light)
# Remove all errors happened before the window start
window_start = time.to_i - light.window_size
window_start = 0 if window_start < 0
transaction.zremrangebyscore(failures_key, 0, window_start)
# Keep at most +light.threshold+ number of errors
transaction.zremrangebyrank(failures_key, 0, -light.threshold - 1)
end
def query_failures(light, transaction: @redis)
window_start = Time.now.to_i - light.window_size
window_start = 0 if window_start < 0
transaction.zrange(failures_key(light), 10_000_000_000, window_start, rev: true, by_score: true)
end
end
end
endMONITOR before: MONITOR after: |
|
It appears the workaround isn't sufficient. I'm seeing Interestingly, I only see it towards AWS ElastiCache, not towards ordinary Redis. Towards ElastiCache: Towards ordinary Redis: |
|
Hey @mt-clearhaus 👋 Thank you for reporting this! irb(main):020:0> redis = Redis.new(...)
irb(main):021:0> redis.zrange('stoplight:v4:failures:primary', 'Infinity', '-Infinity')The snippet you provided as reproduction steps is not equivalent to what Stoplight sends to Redis. Stoplight searches by score, while you're searching by rank. Sets' rank indeed should be an integer, while the score value is a float (see) This is the correct ruby command: redis.zrange('stoplight:v4:failures:primary', 'Infinity', '-Infinity', by_score: true, rev: true)it's translated to Redis command: The command above works with Redis 6.2.13 for sure (we run tests against it) and Redis 7.2.0. May I ask you to test if these two commands work on your version of Redis with and without using the ElastiCache?: |
|
Actually, we can safely replace -Infinity with zero and +Infinity with large enough number because the score represent a timestamp in this case. |
|
I tested it here using Redis 6.2.6 and the specs pass #214 |
|
Hi @bolshakov, Thank you very much for creating Stoplight and for getting back to me. It turns out my issues are only present for AWS ElastiCache. The engine version is only 6.0.5. I have primed both my local Redis 6.2.6 and the AWS ElastiCache with two failures in the Things work flawlessly with the local Redis 6.2.6: irb(main):001:0> redis = Redis.new([...])
=> #<Redis client v5.0.7 for redis://redis:6379/0>
irb(main):002:0> pp redis.zrange('stoplight:v4:failures:primary', 'Infinity', '-Infinity', by_score: true, rev: true)
["{\"error\":{[...]},\"time\":\"2023-08-29T07:20:13.185982997+00:00\"}",
"{\"error\":{[...]},\"time\":\"2023-08-29T07:16:51.963196041+00:00\"}"]
irb(main):003:0> pp redis.zrange('stoplight:v4:failures:primary', 'Inf', '-Inf', by_score: true, rev: true)
["{\"error\":{[...]},\"time\":\"2023-08-29T07:20:13.185982997+00:00\"}",
"{\"error\":{[...]},\"time\":\"2023-08-29T07:16:51.963196041+00:00\"}"]It's another story with AWS ElastiCache: irb(main):001:0> redis = Redis.new([...])
=> #<Redis client v5.0.7 for redis://[...].cache.amazonaws.com:6379/0>
irb(main):002:0> pp redis.zrange('stoplight:v4:failures:primary', 0, 100)
["{\"error\":{[...]},\"time\":\"2023-08-29T07:37:00.265572267+00:00\"}",
"{\"error\":{[...]},\"time\":\"2023-08-29T07:38:00.749749018+00:00\"}"]
irb(main):003:0> redis.zrange('stoplight:v4:failures:primary', 'Infinity', '-Infinity', by_score: true, rev: true)
[...]/redis-client-0.16.0/lib/redis_client/connection_mixin.rb:35:in `call': ERR value is not an integer or out of range (Redis::CommandError)
irb(main):004:0> redis.zrange('stoplight:v4:failures:primary', 'Inf', '-Inf', by_score: true, rev: true)
[...]/redis-client-0.16.0/lib/redis_client/connection_mixin.rb:35:in `call': ERR value is not an integer or out of range (Redis::CommandError)I will try to upgrade the engine version in AWS ElastiCache. |
|
I think it is safe to close this issue as it seems Stoplight is not to blame 🙂 |
|
AWS ElastiCache with engine version 6.2.6 has no issues with the queries 👍 🙌 |
|
Glad you managed to pinpoint the issue!
Right! In Redis 6.2.0, the ZRANGEBYSCORE was deprecated since they introduced the BYSCORE argument for ZRANGE command. It seems this version of AWS ElastiCache does not support the BYSCORE argument yet. Please, keep me updated if you managed to run it with ElastiCache |
|
The syntax error quoted above was with ElastiCache engine version 6.0.5, not 6.2.6. I believe the issue is that the integer As 6.0.5 doesn't support integers (or scores in a sorted set? 🤔) of that size, it wouldn't be possible to workaround Edit: After some thought: You might be right that the syntax error was caused by the |
Describe the bug
Both Redis 6.2.6 (via Docker image) and AWS ElastiCache 6.0.5 complain when the strings
Infinityand-Infinityare used as start or stop value for ZRANGE. For Stoplight 4.1.0 this is the case inStoplight::DataStore::Redis#query_failures:stoplight/lib/stoplight/data_store/redis.rb
Line 139 in 717936c
If no custom window size is set for a light, the default
Float::INFINITYwill be used, and the start and stop values for ZRANGE will become the stringsInfinityand-Infinity, respectively. This is becauseTime.now.to_i - Float::INFINITYequals-Infinity, and because RedisClient's command builder calls#to_son Floats:https://github.com/redis-rb/redis-client/blob/cfbe7feeebe696ee6f5c9a0a27a99bd792b67430/lib/redis_client/command_builder.rb#L34-L35
The error I see is this:
I'm using Ruby 3.1.2p20 on Debian Bookworm.
Redis MONITOR gives me this (the light's name is "primary"):
To Reproduce
Expected behavior
I hope
Float::INFINITYcan be replaced with something else, however, I not sure what the right solution is.Screenshots
N/A
Environment (please complete the following information):
debian:bookworm-slim)Additional context
N/A
The text was updated successfully, but these errors were encountered: