search

phoenixframework / phoenix_pubsub_redis

The Redis PubSub adapter for the Phoenix framework
175 stars 66 forks source link

The redis pubsub adapter seems to be broken with Phoenix 1.3 #31

Closed jfrolich closed 6 years ago

jfrolich commented 6 years ago

My configuration: 

config :cogo, CogoWeb.Endpoint,
  pubsub: [
    name: Cogo.PubSub,
    adapter: Phoenix.PubSub.Redis,
    node_name: "dev_node"
  ]

It seems to connect correctly with redis (no errors)

when I try to broadcast it does not deliver the messages:

Interactive Elixir (1.5.0) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Phoenix.PubSub.subscribe(Cogo.PubSub, "test")
:ok
iex(2)> Phoenix.PubSub.broadcast(Cogo.PubSub, "test", "bla")
:ok
iex(3)> flush
:ok

Is there any major issue with how I tested this? If this is the case, it might be helpful to raise an error somewhere.

mitchellhenke commented 6 years ago

hi @jfrolich! thanks for opening this issue.

It looks like it may be related to Elixir 1.5 rather than Phoenix 1.3. Can you confirm if it works with Elixir 1.4.4?

mitchellhenke commented 6 years ago

Actually, sorry, I was just forgetting to flush in 1.5.1. I'm not able to reproduce this. Can you put together a minimal reproduction of the issue?

jfrolich commented 6 years ago

Yes sure! I updated the elixir version to 1.5.1, didn't help the case. Also on the latest version of phoenix, phoenix_pubsub and phoenix_pubsub_redis. I will create an example app as a test-case.

jfrolich commented 6 years ago

Here a test-case: https://github.com/jfrolich/redis_pubsub

jfrolich commented 6 years ago

I also use redix in my app, where it works, so my redis installation is connecting properly. But perhaps it's a problem with connecting to redis that is not handled.

mitchellhenke commented 6 years ago

That is perfect, thank you! I'll take a look today or tomorrow 🙂

mitchellhenke commented 6 years ago

It appears the issue is related to the overridden version of redix and not overriding redix_pubsub. I put together a PR for updating them in #32. It appears to fix the test case above, if you could confirm it also fixes the issue for you, I'll merge and plan on cutting a release soon.

Thank you again for the report 🙂

jfrolich commented 6 years ago

Hi @mitchellhenke, this actually was an issue that could delay the launch of our product. Thanks for fixing it so quickly, this allows us to have a clean solution to distributed pubsub! 👍

mitchellhenke commented 6 years ago

@jfrolich glad it's been solved!