Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supervised connection #53

Merged
merged 3 commits into from
May 4, 2017
Merged

Supervised connection #53

merged 3 commits into from
May 4, 2017

Conversation

mmmries
Copy link
Collaborator

@mmmries mmmries commented May 4, 2017

Addresses #44

This introduces a new module (with docs) that users can put into their supervision tree to get an automatically reconnecting Gnat connection. They can pass it a list of locations they want to connect to and let it roll.

/cc @newellista @tallguy-hackett @jjcarstens @film42 @quixoten

Copy link
Collaborator

@film42 film42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@quixoten
Copy link
Contributor

quixoten commented May 4, 2017

It might be better to use a single connection settings map that a host and port gets merged into. The broker can send additional cluster endpoints in info messages after the initial handshake, which only includes host and port. Is support for that planned? Which connection settings would get applied to those?

@mmmries
Copy link
Collaborator Author

mmmries commented May 4, 2017

@quixoten I was planning to let client code support that use with something like:

nats_hosts = ["node1", "node2", "node3"]
connection_settings = Enum.map(nats_hosts, fn(host) -> %{host: host, port: 4222, username: "user", password: "pass"} end)
ConnectionSupervisor.start_link(%{name: :gnat, connection_settings: connection_settings})

It seemed easier for the client code to do custom logic for rather than having the library match things up together. Do you think that works or should we support something at the library level?

@mmmries mmmries merged commit 1c9cdbf into master May 4, 2017
@mmmries mmmries deleted the supervised_connection branch May 4, 2017 20:35
@quixoten
Copy link
Contributor

quixoten commented May 4, 2017

So would the INFO messages get forwarded to the client?

@mmmries
Copy link
Collaborator Author

mmmries commented May 4, 2017

I don't follow, are you thinking we would notify another process whenever the connection was established? In a future PR I'm hoping to support persistent subscriptions like RPC server with a model similar to that, but this PR is mostly just about try to keep an gnat connection open whenever possible.

@quixoten
Copy link
Contributor

quixoten commented May 4, 2017

I nats cluster can be completely liquid, for example, in a containerized infrastructure. You might get an initial list of servers from consul, or etcd, but as nodes leave and enter the cluster, the brokers will send additional INFO messages containing additional servers, at least if the client reports that it supports it:

CONNECT
...
protocol: optional int. Sending 0 (or absent) indicates client supports original protocol. Sending 1 indicates that the client supports dynamic reconfiguration of cluster topology changes by asynchronously receiving INFO messages with known servers it can reconnect to.

To support that, I think it would be better to have a shared set of connection settings.

@quixoten
Copy link
Contributor

quixoten commented May 4, 2017

Here's an example from a netcat session. Connected to one node, than added another node to the cluster:

$ netcat 127.0.0.1 14222
INFO {"server_id":"t3QJq4Onv1ROXNMDd2OU9F","version":"0.9.6","go":"go1.7.4","host":"127.0.0.1","port":14222,"auth_required":false,"ssl_required":false,"tls_required":false,"tls_verify":false,"max_payload":1048576} 
CONNECT {"protocol": 1}
+OK
PING
PONG
INFO {"server_id":"t3QJq4Onv1ROXNMDd2OU9F","version":"0.9.6","go":"go1.7.4","host":"127.0.0.1","port":14222,"auth_required":false,"ssl_required":false,"tls_required":false,"tls_verify":false,"max_payload":1048576,"connect_urls":["127.0.0.1:24222"]}

@mmmries
Copy link
Collaborator Author

mmmries commented May 5, 2017

@quixoten I'll open an issue to discuss that. I'm sure we don't currently support it. I think we were only handling INFO messages at startup time, but we will definitely need to add support for receiving them and perhaps collecting that information somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants