go-discover is only used during initial client introduction
- Dominant language
- Go
- Stars
- 17k
- Forks
- 2.1k
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 105
Description
While we were working on https://github.com/hashicorp/nomad/pull/16490 @schmichael and I had a discussion about how we might improve the current process for server discovery and failover with respect to [`go-discover`](https://github.com/hashicorp/go-discover) and Consul. Brainstorming some potential improvements:
1. Punt Consul discovery to `go-discover` and call back into `go-discover` whenever a client can't find a single healthy server. (Probably more effort than its worth and significant backward compat risks.)
2. Persist the server list to client state for faster registrations after node restarts. (This requires ensuring we don't increase our likelihood of hitting an error + long retry which might _increase_ the number of `down` nodes due to restarts in stead of decreasing that!)
3. Clients could request servers extend their heartbeat on graceful shutdown (2x the default? a bit risky)
4. Actually document/design client disco/retry logic?
(4) is appealing as a bare minimum because our current logic predates RFCs and accreted new features in hopes just a bit more code would make it more robust. It's left us in a situation where there's some potential for weird emergent behavior, and it's all very difficult to debug, explain, or test for correctness in suboptimal conditions.
The existing code conflates some distinct properties and operations:
1. "Consul Discovery" is just kind of sprinkled throughout... mostly I think with the intention of using it when we've run out of other options, but I don't know how precise that is and we may be placing far more trust in Consul's availability and correctness than we should!
2. `go-discover` runs once concurrent with Client startup, so I think it races with Consul Discovery and could overwrite a perfectly good server list with a stale one? Again... hard to reason about since there's so many concurrent operations.
3. Our discovery is concurrent with registration is concurrent with other RPCs and that causes tons of complexity: https://github.com/hashicorp/nomad/pull/15808
This needs some discussion and design.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.