Broker discovery #2128
Broker discovery #2128
Conversation
influxdb-denver-pair
force-pushed
the
broker-discovery
branch
from
ca11487 to
fbd8f89
Compare
| @@ -23,7 +23,7 @@ import ( | |||
| "github.com/influxdb/influxdb/udp" | |||
| ) | |||
|
|
|||
| func Run(config *Config, join, version string) (*messaging.Broker, *influxdb.Server) { | |||
| func Run(config *Config, join, version string) (*messaging.Broker, *influxdb.Server, *raft.Log) { | |||
There was a problem hiding this comment.
I feel like the return values are getting a little bit out of control. Perhaps we need a more generic node type which returned by Run(). This node type would have a Close() method, which would take of closing whatever was opened. Sometimes node would have a broker, sometimes a log, sometimes a server, sometimes all, etc.
There was a problem hiding this comment.
Agreed. We started down that path in #2118. We're moving in that direction but planning to do that in a separate PR.
|
Does our design allow a node's URL to change? If so, using a node's URL as a key in a map could be problematic. If it does not allow a node's URL to change, then why continually send it over the heartbeat channel? |
jwilder
force-pushed
the
broker-discovery
branch
2 times, most recently
from
5d471a3 to
9013842
Compare
|
@otoolep Updated based on your feedback. |
jwilder
force-pushed
the
broker-discovery
branch
from
9013842 to
90b0057
Compare
| mu sync.Mutex | ||
| path string // config file path | ||
| conns []*Conn // all connections opened by client | ||
| url url.URL // current known leader URL |
There was a problem hiding this comment.
This was there before, but we need to come back and fix this naming. url and urls are not great names on the same object. As names they don't mean much, and are too similar. They need to become leaderURL and brokerURLs.
Not something required for this PR, one of us can fix it up afterwards.
|
OK, makes sense to me. Just some nits, which I don't need to re-review. You may wish to get this blessed by @benbjohnson, whatever you think. Don't forget the CHANGELOG -- you could probably call it a "feature". |
jwilder
force-pushed
the
broker-discovery
branch
from
90b0057 to
570c3f3
Compare
| @@ -84,6 +88,7 @@ func TestRestoreCommand(t *testing.T) { | |||
| f.Close() | |||
|
|
|||
| // Stop server. | |||
| l.Close() | |||
| s.Close() | |||
| b.Close() | |||
There was a problem hiding this comment.
Can you defer the closing right after you validate that b, s, & l are non-nil?
This sends data node urls via the broker heartbeat from each data node. The urls are tracked on the broker to support simpler cluster setup as well as distributed queries.
jwilder
force-pushed
the
broker-discovery
branch
from
570c3f3 to
cf9e0b2
Compare
fix(platform): remove creator and time
This sends data node urls via the broker heartbeat from each data node. The urls are tracked on the broker to support simpler cluster setup as well as distributed queries.
This is a prerequisite for splitting data and broker nodes.
cc @benbjohnson @otoolep