Add simple DNS clustering #177
Conversation
|
|
||
| # discover nodes on init so we're connected before downstream children start | ||
| state = | ||
| do_discovery(%{ |
There was a problem hiding this comment.
we can later introduce a :sync_init option or similar, but I feel like it's best to do synchronous discovery on init w/ timeout, to lessen races on downstream children starting without full mesh connections
| @@ -83,6 +98,16 @@ defmodule Phoenix.PubSub.PG2 do | |||
| Supervisor.child_spec({Phoenix.PubSub.PG2Worker, {name, group}}, id: group) | |||
| end | |||
|
|
|||
| children = | |||
| case System.get_env("PHX_CLUSTER_DNS") do | |||
There was a problem hiding this comment.
We really shouldn't be relying on env vars down here. Also, the cluster name is not related to PubSub, so I would say this functionality is out.
My suggestion is to add Phoenix.PubSub.DNSCluster as a service and have your Phoenix endpoint start it based on a config:
config :my_app, MyApp.Endpoint, dns_cluster: true
There was a problem hiding this comment.
It's not "related", but also the pg adapter is not useful if you aren't clustered, so it's still pretty well related. I'm fine with the endpoint starting it as well, but we have a problem in that the pubsub adapter is started above the endpoint for good reasons, so we'd be starting pubsub before clustering, which seems backwards, so I'm not sure?
There was a problem hiding this comment.
So to put another way, I'm convinced we need a knob free way to get out of the box clustering, but if we don't put in the pubsub tree we don't have any other option to start it in the correct order, short of adding it explicitly to the children in phx.new
| use GenServer | ||
| require Logger | ||
|
|
||
| defmodule Resolver do |
There was a problem hiding this comment.
Why is it a separate module? 🤔
There was a problem hiding this comment.
just for testing. Alternative is doc false'ing everything on the public DNSCluster
|
Closing in favor of separate library https://github.com/phoenixframework/dns_cluster |
No description provided.