Implement the process registry API for RedixSentinel

[josevalim]

Today, developers that want to use Redix or RedixSentinel have to keep an interface against both APIs. For example, if I start a Redix connection, then I need to call Redix.command or Redix.pipeline. If I start RedixSentinel, then I need to use RedixSentinel.command or RedixSentinel.pipeline. This ends up pushing complexity to the users of those projects.

I believe one possible approach to solve this problem is to define the process registry API for RedixSentinel. This way, developers will be able to do:

# when using redix
Redix.command(conn_pid, [...])

# when using redix_sentinel
Redix.command({:via, RedixSentinel, sentinel_pid}, [...])

For this to work, RedixSentinel needs to implement the following functions (this is a proof of concept):

@doc false
def whereis_name(sentinel_pid) do
  case Connection.call(sentinel_pid, :node) do
    {:ok, conn_pid} -> conn_pid
    {:error, reason} -> raise ArgumentError, "something something"
  end
end

@doc false
def send(sentinel_pid, msg) do
  Kernel.send(whereis_name(sentinel_pid), msg)
end

@doc false
def register_name(_, _) do
  raise ArgumentError, "cannot use :via for registration with RedixSentinel"
end

I haven't tried it but I think it may work.

Thoughts?

Thanks for your time!

[ananthakumaran]

Agree, there is definitely some friction compared to other language libraries where sentinel could be used with just a config change. I am on vacation now. Will try to check this after the vacation.

On Thu, Oct 11, 2018, 12:12 AM José Valim notifications@github.com wrote:

Today, developers that want to use Redix or RedixSentinel have to keep an interface against both APIs. For example, if I start a Redix connection, then I need to call Redix.command or Redix.pipeline. If I start RedixSentinel, then I need to use RedixSentinel.command or RedixSentinel.pipeline. This ends up pushing complexity to the users of those projects.

I believe one possible approach to solve this problem is to define the process registry API for RedixSentinel. This way, developers will be able to do:

when using redix

Redix.command(conn_pid, [...])

when using redix_sentinel

Redix.command({:via, RedixSentinel, sentinel_pid}, [...])

For this to work, RedixSentinel needs to implement the following functions (this is a proof of concept):

@doc falsedef whereis_name(sentinel_pid) do case Connection.call(sentinel_pid, :node) do {:ok, conn_pid} -> conn_pid {:error, reason} -> raise ArgumentError, "something something" endend @doc falsedef send(sentinel_pid, msg) do Kernel.send(whereis_name(sentinel_pid), msg)end @doc falsedef registername(, _) do raise ArgumentError, "cannot use :via for registration with RedixSentinel"end

I haven't tried it but I think it may work.

Thoughts?

Thanks for your time!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ananthakumaran/redix_sentinel/issues/4, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJG9mRkBtQviun09zvjN7y1aUxSZ-0_ks5ujj-agaJpZM4XWFk- .

[ananthakumaran]

I just tried this approach, it works with the normal case. But I am not sure about how to handle errors as before.

Currently, RedixSentinel spawns a Redix process(with :exit_on_disconnection option) and monitors it. Redix connection will exit whenever the underlying tcp connection is closed. RedixSentinel will then try the restart protocol and find a new redis server to connect.

When the restart is going on, RedixSentinel will return {:error, %Redix.ConnectionError{reason: :closed}} for all client calls. This is essential to mimic the same behaviour as Redix, which also returns the same error when it tries to re-establish the tcp connection.

With registry, it seems like there is no way to control what is returned to the client when the Redix connection is not established.

[josevalim]

@ananthakumaran how do you know the connection is closed? Is it when you attempt to run a command then it turns out it is closed? Or is Redix actively pinging the server? If the latter, we could quickly ask the connection if it is still alive and that should be enough for the huge majority of cases, otherwise I think you are right, there is not much we can do. :(

[ananthakumaran]

When the connection is closed (according to Redix), it will exit and RedixSentinel will get the :down message because of the monitor. As for Redix, AFAIK it only knows the connection is dead when it tries to use the socket.

  def command!(conn, command, opts \\ []) do
    with_node(conn, fn node ->
      Redix.command!(node, command, opts)
    end)
  end

  defp with_node(conn, callback) do
    with {:ok, node} <- Connection.call(conn, :node) do
      try do
        callback.(node)
      catch
        :exit, {:noproc, _} -> {:error, %Redix.ConnectionError{reason: :closed}}
        :exit, {%Redix.ConnectionError{} = reason, _} -> {:error, reason}
      end
    end
  end

Because of the indirection involved, there are more edgecases. RedixSentinel handles it by catching exits.

we could quickly ask the connection if it is still alive and that should be enough for the huge majority of cases

Let's assume we know the Redix process is not alive inside the whereis_name, but we could only return PID or raise Exception. How would that allow us to control the return value of Redix.command.

[josevalim]

You are correct, it wouldn't be possible to change the error semantics. I think this was a red flag after all then. Thanks for looking into it! :heart: