derekkraan / horde

Horde is a distributed Supervisor and Registry backed by DeltaCrdt
MIT License
1.32k stars 106 forks source link

GenServer.cast cant find process via {:via, Horde.Registry, ...} #248

Closed Ellocsys closed 2 years ago

Ellocsys commented 2 years ago

Hi! GenServer.cast/2 cant find process via {:via, Horde.Registry, ...} In case cast using pid everything works ok I prepared a small test to explain the problem

defmodule HordeTest do
  use ExUnit.Case

  test "via with cast" do
    horde = start_registry()

    name = {:via, Horde.Registry, {horde, "foo", %{data: "data"}}}

    {:ok, pid} = Agent.start_link(fn -> 0 end, name: name)

    assert Agent.cast(name, &(&1 + 1)) == :ok

    assert Agent.cast(pid, &(&1 + 3)) == :ok

    Process.sleep(100)

    # Fail because Agent returns 3
    assert Agent.get(name, & &1) == 4
  end

  defp start_registry(opts \\ [keys: :unique]) do
    horde = :"h#{-:erlang.monotonic_time()}"

    {:ok, _pid} =
      Horde.Registry.start_link([name: horde, delta_crdt_options: [sync_interval: 20]] ++ opts)

    horde
  end
end

Is this behaviour would be a bug?

Horde: 0.8.6 Elixir: 1.12.3 Erlang: 24.1.6

derekkraan commented 2 years ago

Hi,

Does the test pass if you substitute Horde.Registry for Registry?

derekkraan commented 2 years ago

In the docs for Agent: https://hexdocs.pm/elixir/1.13/Agent.html#t:agent/0

https://hexdocs.pm/elixir/1.13/Agent.html#cast/2

Agent does not support the via tuple.

smaximov commented 2 years ago

@derekkraan, looks like it does:

agent() :: pid() | {atom(), node()} | name()
name() :: atom() | {:global, term()} | {:via, module(), term()}

i.e. agent()name(){:via, module(), term()}

smaximov commented 2 years ago

It works for me in an IEx session, so maybe it has something to do with the tests setup:

$ iex -S mix run --no-start       
Erlang/OTP 24 [erts-12.1.4] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit]

Interactive Elixir (1.12.3) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Application.ensure_all_started(:horde)
{:ok, [:merkle_map, :delta_crdt, :libring, :telemetry_poller, :horde]}
iex(2)> Horde.Registry.start_link(name: HR, keys: :unique)

18:05:05.174 [info]  Starting Horde.RegistryImpl with name HR
{:ok, #PID<0.1768.0>}
iex(3)> name = {:via, Horde.Registry, {HR, "name"}}
{:via, Horde.Registry, {HR, "name"}}
iex(4)> {:ok, agent} = Agent.start_link(fn -> 0 end, name: name)
{:ok, #PID<0.1773.0>}
iex(5)> Agent.get(name, & &1)
0
iex(6)> Agent.cast(agent, &(&1 + 1))
:ok
iex(7)> Agent.get(name, & &1)
1
iex(8)> Agent.cast(name, &(&1 + 1))
:ok
iex(9)> Agent.get(name, & &1)
2
smaximov commented 2 years ago

So we investigated a little and found that my example works because the via tuple doesn't contain any metadata; and removing the metadata from the via tuple when casting the agent process fixes the original test. Changing Horde.Registry to Registry doesn't seem to make any difference, so it's probably not a Horde issue.

The interesting thing is that changing Agent.cast/2 to Agent.call/2 works regardless using metadata in the via tuple. I've opened an issue in the Elixir's repo if you are interested in details.

derekkraan commented 2 years ago

Great thanks for looking into this!