rpc: automatically maintain RPC connections across cluster #70111
Labels
A-kv-server
Relating to the KV-level RPC server
A-server-networking
Pertains to network addressing,routing,initialization
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-kv
KV Team
Projects
Comments
erikgrinaker
added
C-enhancement
This was referenced Sep 13, 2021
|
Would be nice to address this while here. I think I was seeing this happen for an internal support case. cockroach/pkg/rpc/nodedialer/nodedialer.go Lines 191 to 196 in 793b4c8 |
tbg
added a commit
to tbg/cockroach
that referenced
this issue
Sep 30, 2021
See cockroachdb#68419 (comment) for the original discussion. This commit adds a new `circuit` package that uses probing-based circuit breakers. This breaker does *not* recruit the occasional request to carry out the probing. Instead, the circuit breaker is configured with an "asychronous probe" that effectively determines when the breaker should reset. We prefer this approach precisely because it avoids recruiting regular traffic, which is often tied to end-user requests, and led to inacceptable latencies there. The potential downside of the probing approach is that the breaker setup is more complex and there is residual risk of configuring the probe differently from the actual client requests. In the worst case, the breaker would be perpetually tripped even though everything should be fine. This isn't expected - our two uses of circuit breakers are pretty clear about what they protect - but it is worth mentioning as this consideration likely influenced the design of the original breaker. Touches cockroachdb#69888 Touches cockroachdb#70111 Touches cockroachdb#53410 Also, this breaker was designed to be a good fit for: cockroachdb#33007 Release note: None
We no longer believe that this was the issue in the referenced support case. |
tbg
added a commit
to tbg/cockroach
that referenced
this issue
Oct 1, 2021
See cockroachdb#68419 (comment) for the original discussion. This commit adds a new `circuit` package that uses probing-based circuit breakers. This breaker does *not* recruit the occasional request to carry out the probing. Instead, the circuit breaker is configured with an "asychronous probe" that effectively determines when the breaker should reset. We prefer this approach precisely because it avoids recruiting regular traffic, which is often tied to end-user requests, and led to inacceptable latencies there. The potential downside of the probing approach is that the breaker setup is more complex and there is residual risk of configuring the probe differently from the actual client requests. In the worst case, the breaker would be perpetually tripped even though everything should be fine. This isn't expected - our two uses of circuit breakers are pretty clear about what they protect - but it is worth mentioning as this consideration likely influenced the design of the original breaker. Touches cockroachdb#69888 Touches cockroachdb#70111 Touches cockroachdb#53410 Also, this breaker was designed to be a good fit for: cockroachdb#33007 which will use the `Signal()` call. Release note: None
|
There's a push to re-work some of the rpc circuit breaking which is outlined here #70485 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, components must themselves dial remote nodes whenever they interact with them, to make sure a connection is established. This mostly applies to Raft, DistSender, DistSQL, and the closed timestamp side transport. These components will also interact with the RPC health checks and circuit breakers.
This can be problematic because it can introduce very high latency (tens of seconds) when interacting with unresponsive nodes (e.g. when the server/VM is shut down or during network unavailability), which is not acceptable for many performance-critical code paths (see #70017). It also muddies the water wrt. who is responsible for running health checks and circuit breaker probes (see #68419).
Instead, we should have a single actor (i.e. goroutine) responsible for maintaining RPC connections to other nodes and performing health checks. The connection health should be exposed in such a way that RPC clients can fail fast whenever they try to interact with a known-bad node.
@tbg has some thoughts on the health check/breaker issues in #68419 (comment), but we should consider extending that proposal to also manage all RPC connections by dialing remote nodes as appropriate.
/cc @cockroachdb/kv
Jira issue: CRDB-9948
gz#13169
Epic CRDB-32137
The text was updated successfully, but these errors were encountered: