beam-telemetry / telemetry_metrics_statsd

Telemetry.Metrics reporter for StatsD-compatible metric servers
https://hexdocs.pm/telemetry_metrics_statsd
MIT License
75 stars 44 forks source link

Consider using ETS or :persistent_term #33

Closed josevalim closed 4 years ago

josevalim commented 4 years ago

To reduce contention, maybe the socket can be put into a persistent_term?

josevalim commented 4 years ago

Another option is to just register the socket directly: Process.register(conn.sock, __MODULE__). There is a concern that maybe a pool is necessary (one was recently introduced in Statix), which means that persistent_term may be the best way to go.

juanperi commented 4 years ago

I ran a quick benchmark comparing current state, ets with a single value, and ets with a naive implementation of a pool with size 40

  def init(config) do
    ........
    :ets.new(__MODULE__, [:set, :protected, :named_table])
    ports =
      for _ <- 1..config.pool_size do
        {:ok, port} = UDP.open(udp_config)
        port
      end
    :ets.insert(__MODULE__, {:ports, ports})
    .........
  def get_udp(reporter) do
    [{_, values}] = :ets.lookup(__MODULE__, :ports)
    Enum.random(values)
  end

and the results are:

$ mix run benchmark.exs
Operating System: macOS
CPU Information: Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
Number of Available Cores: 8
Available memory: 16 GB
Elixir 1.10.1
Erlang 22.2.7

Benchmark suite executing with the following configuration:
warmup: 1 s
time: 10 s
memory time: 0 ns
parallel: 100
inputs: none specified
Estimated total run time: 33 s

Benchmarking ets...
Benchmarking original...
Benchmarking pool...

Name               ips        average  deviation         median         99th %
pool            1.86 K      537.24 μs   ±229.25%          35 μs        5072 μs
ets             1.18 K      844.90 μs    ±22.56%         811 μs        1382 μs
original        1.01 K      990.98 μs    ±19.14%         977 μs        1469 μs

Comparison:
pool            1.86 K
ets             1.18 K - 1.57x slower +307.66 μs
original        1.01 K - 1.84x slower +453.74 μs

At this point, it looks like the pool is the most performant version. But it will depend on how smart do we need to make the pool. Do we consider that opening a port is always successful? That once opened the port never dies? if we do consider that a port might die, we would have to implement locking and a checkout mechanism. At this point it might be better to reach for an already existing pool implementation, instead of reimplementing here. If we do reach out for an external library, maybe the benefits will not be as important as in this example

juanperi commented 4 years ago

hey @arkgil do you have any preference on the path forward here?

arkgil commented 4 years ago

@epilgrim I would prefer to not make an assumption that the socket never dies. We should keep the current logic which detects a UDP error and opens the new socket when it happens. If we can have a pool that satisfies this constraint, that would be great.