-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Async mode not compatible with Redis logical databases #135
Comments
There is an example protocol wrapper upstream with TLS and logical databases support. https://github.com/socketry/async-redis/blob/main/examples/auth/wrapper.rb Works like a charm for me, like this: diff --git a/lib/3scale/backend/storage_async/client.rb b/lib/3scale/backend/storage_async/client.rb
index fc0c0a4a..ebdcb108 100644
--- a/lib/3scale/backend/storage_async/client.rb
+++ b/lib/3scale/backend/storage_async/client.rb
@@ -205,8 +205,7 @@ module ThreeScale
host = uri.host || DEFAULT_HOST
port = uri.port || DEFAULT_PORT
- endpoint = Async::IO::Endpoint.tcp(host, port)
- Async::Redis::Client.new(endpoint, limit: opts[:max_connections])
+ AsyncRedisClientWrapper.connect(uri, limit: opts[:max_connections])
end
def init_sentinels_client(opts)
@@ -217,6 +216,73 @@ module ThreeScale
Async::Redis::SentinelsClient.new(name, opts[:sentinels], role)
end
end
+
+ class AsyncRedisClientWrapper
+ class << self
+ # @param url [String] Redis URL connection string
+ # @param ssl_params [Hash] passed to OpenSSL::SSL::SSLContext
+ # @param options [Hash] passed to Async::Redis::Client.new
+ # @return [Async::Redis::Client]
+ def call(url = 'redis://localhost:6379', ssl_params: nil, **options)
+ uri = URI(url)
+
+ endpoint = prepare_endpoint(uri, ssl_params)
+
+ credentials = []
+ credentials.push(uri.user) if uri.user && !uri.user.empty?
+ credentials.push(uri.password) if uri.password && !uri.password.empty?
+
+ db = uri.path[1..-1].to_i if uri.path
+
+ protocol = AsyncRedisProtocolWrapper.new(db: db, credentials: credentials)
+
+ Async::Redis::Client.new(endpoint, protocol: protocol, **options)
+ end
+
+ alias :connect :call
+
+ # @param uri [URI]
+ # @param ssl_params [Hash]
+ # @return [Async::IO::Endpoint]
+ def prepare_endpoint(uri, ssl_params = nil)
+ tcp_endpoint = Async::IO::Endpoint.tcp(uri.hostname, uri.port)
+ case uri.scheme
+ when 'redis'
+ tcp_endpoint
+ when 'rediss'
+ ssl_context = OpenSSL::SSL::SSLContext.new
+ ssl_context.set_params(ssl_params) if ssl_params
+ Async::IO::SSLEndpoint.new(tcp_endpoint, ssl_context: ssl_context)
+ else
+ raise ArgumentError
+ end
+ end
+ end
+ end
+
+ class AsyncRedisProtocolWrapper
+ def initialize(db: 0, credentials: [], protocol: Async::Redis::Protocol::RESP2)
+ @db = db
+ @credentials = credentials
+ @protocol = protocol
+ end
+
+ def client(stream)
+ client = @protocol.client(stream)
+
+ if @credentials.any?
+ client.write_request(["AUTH", *@credentials])
+ client.read_response
+ end
+
+ if @db
+ client.write_request(["SELECT", @db])
+ client.read_response
+ end
+
+ return client
+ nd
+ end
end
end
end
So I guess we can add this. Just don't know if it's going to work with sentinels. On the other hand idk if sentinels support logical databases to begin with. Anyway, needs some investigation and testing but it seems to be straightforward overall. |
The async-redis client does not support Redis logical databases. That means that it does not work properly on environments that have for example, the main db in
redis://redis-backend:6379/0
and the queues inredis://redis-backend:6379/1
: https://github.com/3scale/3scale-operator/blob/master/pkg/3scale/amp/auto-generated-templates/amp/amp.ymlRef: socketry/async-redis#15
The text was updated successfully, but these errors were encountered: