hammer-backend-redis icon indicating copy to clipboard operation
hammer-backend-redis copied to clipboard

Having trouble connecting with Redis on prod.

Open talhaazeemmughal opened this issue 1 year ago • 5 comments

Describe the bug i have redix library defined in my config and i also have hammer one defined as below:

config :hammer,
  backend:
    {Hammer.Backend.Redis,
     [
       expiry_ms: :timer.hours(24),
       redix_config: [
         host: System.get_env("HOST", "127.0.0.1"),
         port: System.get_env("PORT", "6379") |> String.to_integer(),
         password: System.get_env("PASSWORD"),
         ssl: System.get_env("SSL", "false") == "true"
       ],
       pool_size: 30,
       timeout: 5000
     ]}

config :redix,
  url: System.get_env("REDIS_URL", "redis://127.0.0.1:6379"),
  pool_size: 30

i am also adding this to redix and hammer library while booting the app for socket_opts:

[
      customize_hostname_check: [
        match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
      ]
]

now redix is getting connected properly as i ran redix ping command i got pong in response but when i execute Hammer.check_rate in prod iex. it gives redis error saying connection closed. i am kind of stuck here.

** Provide the following details

  • Elixir version (elixir -v): 1.10.4-otp-22
  • Erlang version (erl -v): 22.3.4.22
  • Redis version (redis-server -v): 7.0.7
  • Operating system: MAC OS
  • hammer_backed_redis: 6.1.0

Expected behavior Hammer.check_rate should be connecting with redis and giving the required output like {:allow, count}, {:deny, limit}

Actual behavior It actually gives the error tuple {:error, err} and err variable says redis connection closed.

talhaazeemmughal avatar Aug 13 '24 09:08 talhaazeemmughal

i think i need to pass this (since i am using AWS Elasticache Redis Instance)

[
      customize_hostname_check: [
        match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
      ]
]

to the redix_config that i am passing to the hammer library, how can i do that? image

talhaazeemmughal avatar Aug 13 '24 19:08 talhaazeemmughal

Hello! interesting! I use also Elasticache and we don t have that issue. Maybe depends on some configuration you might have setup with the Elasticache but that seems to make sense . If you have time to make an MR, feel free to open one. I ll see tomorrow if I can block sometime to create one and release something

epinault avatar Aug 15 '24 00:08 epinault

Hello! interesting! I use also Elasticache and we don t have that issue. Maybe depends on some configuration you might have setup with the Elasticache but that seems to make sense . If you have time to make an MR, feel free to open one. I ll see tomorrow if I can block sometime to create one and release something

@epinault also i think generating MR will not help me since i am on elixir 1.10.4-otp-22 and hammer 6.2.1 is using elixir 1.13 while the hammer_backend_redis 6.1.2 is using elixir 1.12 and we are upgrading it to use elixir 1.12.

another thing to note over here is that if i pass verify: :verify_none in socket_opts then it works fine.

talhaazeemmughal avatar Aug 15 '24 09:08 talhaazeemmughal

@epinault , I updated the elixir, erlang and hammer libs:

  • Elixir version (elixir -v): 1.13.4-otp-24
  • Erlang version (erl -v): 24.0
  • Redis version (redis-server -v): 7.0.7
  • Operating system: MAC OS
  • hammer_backed_redis: 6.1.2
  • hammer: 6.2.1

and i still have the same issue.

talhaazeemmughal avatar Sep 23 '24 11:09 talhaazeemmughal

do you have more details on the error? cause this is networking issue and I am not sure why that would fail. does it have that error all the time?

epinault avatar Sep 25 '24 21:09 epinault

do you have more details on the error? cause this is networking issue and I am not sure why that would fail. does it have that error all the time?

@epinault I get this error only when i execute Hammer.check_rate for other redix library is working fine and is getting connected with redis.

talhaazeemmughal avatar Sep 26 '24 15:09 talhaazeemmughal

@epinault if i pass verify: :verify_none in socket_opts then it works fine.

talhaazeemmughal avatar Sep 27 '24 08:09 talhaazeemmughal

This is more likely an issue with redux itself or whether you have SSL properly setup. Do you have the certificate setup correctly? You need the proper set of certificate likely installed if you want verify on for ssl.

But might be worth asking on the redix package see what they say?

epinault avatar Sep 27 '24 14:09 epinault

Also know that the PONG command in cluster is not implemented in redix, there is a ticket out there in that project too

epinault avatar Sep 27 '24 14:09 epinault

@epinault I already have an issue on that repo. have a look

talhaazeemmughal avatar Sep 27 '24 14:09 talhaazeemmughal

I see! Thanks that helps! Should have mentioned that early on. I ll See if I can upgrade it early next week

epinault avatar Sep 27 '24 14:09 epinault

I see! Thanks that helps! Should have mentioned that early on. I ll See if I can upgrade it early next week

@epinault so you understand the issue?

talhaazeemmughal avatar Sep 27 '24 15:09 talhaazeemmughal

maybe. seem they are pointing to an older version of Redix. I ll track this down and see if I can loosen up or update the version soon

epinault avatar Sep 30 '24 21:09 epinault

I am looking at the deps I don t see why you have redix 0.11. Hammer redis require 1.1. at a minimum. did you try updating redix ? what is your package version for redix?

epinault avatar Oct 01 '24 15:10 epinault

I am looking at the deps I don t see why you have redix 0.11. Hammer redis require 1.1. at a minimum. did you try updating redix ? what is your package version for redix?

@epinault i have updated the libraries recently and i am using redix 1.5.1 and i am still facing this issue

talhaazeemmughal avatar Oct 01 '24 15:10 talhaazeemmughal

your issue is likely related to https://github.com/whatyouhide/redix/issues/240

epinault avatar Oct 01 '24 15:10 epinault

your issue is likely related to whatyouhide/redix#240

@epinault but it is working fine with exq library as well as redix. It only gives me trouble with hammer.

talhaazeemmughal avatar Oct 01 '24 15:10 talhaazeemmughal

yea I am not sure what is going on. if you check the init

 def init(args) do
    expiry_ms = Keyword.get(args, :expiry_ms)

    if !expiry_ms do
      raise RuntimeError, "Missing required config: expiry_ms"
    end

    key_prefix = Keyword.get(args, :key_prefix, "Hammer:Redis:")

    redix_config =
      Keyword.get(
        args,
        :redix_config,
        Keyword.get(args, :redis_config, [])
      )

    redis_url = Keyword.get(args, :redis_url, nil)

    {:ok, redix} =
      if is_binary(redis_url) && byte_size(redis_url) > 0 do
        Redix.start_link(redis_url, redix_config)
      else
        Redix.start_link(redix_config)
      end

    delete_buckets_timeout = Keyword.get(args, :delete_buckets_timeout, 5000)

    {:ok,
     %{
       redix: redix,
       expiry_ms: expiry_ms,
       delete_buckets_timeout: delete_buckets_timeout,
       key_prefix: key_prefix
     }}
  end

it s looking pretty correct ot pass options..

can you share snippet of code how you started? I see the config but you seem to override both? and you don t need to do that...

epinault avatar Oct 01 '24 16:10 epinault

@epinault from what I have understood so far, if i pass verify: :verify_none in socket_opts then it works fine. I also saw that the socket opts are not getting passed when hammer tries to start the redis instance.

talhaazeemmughal avatar Oct 01 '24 16:10 talhaazeemmughal

@epinault for rest of the libs like exq and redix itself. When i am starting them i am adding socket_opts as:

[
      customize_hostname_check: [
        match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
      ]
    ]

talhaazeemmughal avatar Oct 01 '24 16:10 talhaazeemmughal

also this is how i started redix itself:

talhaazeemmughal avatar Oct 01 '24 16:10 talhaazeemmughal

@epinault this is the codebase:

defp configure_redis_ssl(redis_url) do
    socket_opts = redis_socket_opts(redis_url)

    Application.put_env(
      :exq,
      :redis_options,
      socket_opts: socket_opts
    )

    Application.put_env(
      :redix,
      :socket_opts,
      socket_opts
    )

    # Retrieve the current hammer configuration
    current_hammer_config = Application.get_env(:hammer, :backend)

    # Extract and update the `redix_config`
    {backend_module, backend_opts} = current_hammer_config

    # Update the redix_config with socket_opts
    updated_redix_config =
      backend_opts
      |> Keyword.get(:redix_config, [])
      |> Keyword.put(:socket_opts, socket_opts)

    # Rebuild the backend_opts with the updated redix_config
    updated_backend_opts = Keyword.put(backend_opts, :redix_config, updated_redix_config)

    # Put the updated configuration back into the application environment
    Application.put_env(:hammer, :backend, {backend_module, updated_backend_opts})

    :ok
  end

  # If SSL is enabled for the Redis connection, add additional socket_opts
  # in order to support AWS ElastiCache's wildcard SSL certificates
  defp redis_socket_opts("rediss://" <> _rest) do
    [
      customize_hostname_check: [
        match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
      ]
    ]
  end

  defp redis_socket_opts(_non_ssl_url), do: []

I was not doing it for hammer before but it was not working at that time as well and now that i am actually passing it. I think hammer is not using while starting redix instance.

talhaazeemmughal avatar Oct 01 '24 16:10 talhaazeemmughal

I am a little concern with. -> # Put the updated configuration back into the application environment Application.put_env(:hammer, :backend, {backend_module, updated_backend_opts})

this is an anti pattern in Elixir to be honest. Best is to get the config once from your application, and in your application.ex do the proper passing of the startup

If you look at the code I posted, I don t do anything about socket_opts. And it should be passed like any other fine as I am merely just passing the set of options you pass to Redix itself


what is concerning you are missing bunch of config together ->

Application.put_env(
  :redix,
  :socket_opts,
  socket_opts
)

I would encourage to manage the options a little differently into a config that only read but not put . Redix itself does not need to be configured via the application.put_env

epinault avatar Oct 01 '24 16:10 epinault

I am a little concern with. -> # Put the updated configuration back into the application environment Application.put_env(:hammer, :backend, {backend_module, updated_backend_opts})

this is an anti pattern in Elixir to be honest. Best is to get the config once from your application, and in your application.ex do the proper passing of the startup

If you look at the code I posted, I don t do anything about socket_opts. And it should be passed like any other fine as I am merely just passing the set of options you pass to Redix itself


what is concerning you are missing bunch of config together ->

Application.put_env(
  :redix,
  :socket_opts,
  socket_opts
)

I would encourage to manage the options a little differently into a config that only read but not put . Redix itself does not need to be configured via the application.put_env

if i remove this

# Retrieve the current hammer configuration
    current_hammer_config = Application.get_env(:hammer, :backend)

    # Extract and update the `redix_config`
    {backend_module, backend_opts} = current_hammer_config

    # Update the redix_config with socket_opts
    updated_redix_config =
      backend_opts
      |> Keyword.get(:redix_config, [])
      |> Keyword.put(:socket_opts, socket_opts)

    # Rebuild the backend_opts with the updated redix_config
    updated_backend_opts = Keyword.put(backend_opts, :redix_config, updated_redix_config)

    # Put the updated configuration back into the application environment
    Application.put_env(:hammer, :backend, {backend_module, updated_backend_opts})

it still gives me issues for hammer and doing that for redix and exq is working fine since they are using the socket_opts that i passed to get connected with the AWS Elasticache instance

talhaazeemmughal avatar Oct 01 '24 16:10 talhaazeemmughal

what does "current_hammer_config" do?

epinault avatar Oct 01 '24 16:10 epinault

what does "current_hammer_config" do?

current hammer config is:

config :hammer,
  backend:
    {Hammer.Backend.Redis,
     [
       expiry_ms: :timer.hours(24),
       redix_config: [
         host: System.get_env("REDIS_HOST", "127.0.0.1"),
         port: System.get_env("REDIS_PORT", "6379") |> String.to_integer(),
         password: System.get_env("REDIS_PASSWORD"),
         ssl: System.get_env("REDIS_SSL", "false") == "true"
       ],
       pool_size: 30,
       timeout: 5000
     ]}

i was just passing redis_url before but then added redix_config. I am fetching the backend values and then updating the redix_config with the socket_opts

talhaazeemmughal avatar Oct 01 '24 16:10 talhaazeemmughal

@epinault Any update on your end?

talhaazeemmughal avatar Oct 07 '24 12:10 talhaazeemmughal

I don t have much update. I am looking at Redix and the code I pointed out seems to work fine. I have feeling that there is something specific to how you configure it that messes with it like the put_env and how redix would read that on startup

The company I work for uses it in production and we do not see any issues either with connecting

epinault avatar Oct 07 '24 21:10 epinault

@epinault I found one thing by digging into the hammer code was that I told you initially as well that when hammer is initiating the redix instance. it is not adding this in the socket_opts:

[
      customize_hostname_check: [
        match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
      ]
    ]

talhaazeemmughal avatar Oct 08 '24 14:10 talhaazeemmughal

I cannot spot the problem at the moment but if you see it, feel free to suggest an MR with a fix .

As you can see the redis backend passes the options straight to redix. So as long as the init gets all the args it should just work.

I am not sure I ll have time today and definitely won't for the next few days due to work commitment. In the meantime showing a repro will speed up the resolution too. I only see snippet of your code and I see things that are a bit of an anti pattern already in the configuration. I want to help but a bit ensure what you see compare to what i see

epinault avatar Oct 08 '24 15:10 epinault