stability: real-world testing of proposal quota

[petermattis]

From discussion spawned out of #8639, we need to introduce a flow control mechanism for admitting write operations to replicas. If writes are being applied to a replica sufficiently fast, the raft log might be growing faster than we can generate and apply a snapshot. If that situation arises we'll get a loop of continuous snapshot generation and application which is a drain on the system (and, in effect, throttles all writes). Adjusting the Raft log truncation heuristics (again) is not sufficient as applying a sufficiently large chunk of Raft log entries is slower than using a snapshot.

One idea for a flow control mechanism is to throttle incoming write operations based on the size of the Raft log. A small Raft log indicates that the replicas are all keeping up. As the Raft log grows closer to its target max size (currently the replica size) we would want to throttle writes. I haven't thought of a specific heuristic to use, but am thinking we'd want something that incorporated the excess "raft log capacity" (the delta between the current raft log size and its target max size).

Cc @cockroachdb/stability

[petermattis]

I have a proof of concept for flow control, but it leaves a lot to be desired. The gist of the approach is to maintain a proposal quota pool that we "acquire" quota from before submitting a proposal and "release" quota back to when all of the live replicas have indicated they have committed the proposal. In addition to being fragile, this approach introduces performance blips when a follower node goes down. Without this flow control, when a follower goes down the range is unaffected. With the flow control turned on, when a follower goes down we quickly run out of quota a block new operations on the range until the node is marked dead. I haven't even plumbed through the liveness stuff, but already I'm not happy with this approach.

[petermattis]

Cc @spencerkimball

[spencerkimball]

Let's start a new issue for the flow control.

…

On Mon, Feb 27, 2017 at 12:34 PM Peter Mattis ***@***.***> wrote: Cc @spencerkimball <https://github.com/spencerkimball> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8659 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF3MTbFncE3R5zwYIIuUkAS3nbQheGnEks5rgzNzgaJpZM4JnwJU> .

[petermattis]

Let's start a new issue for the flow control.

Eh? That's what this issue is (or intended to be).

[petermattis]

A different approach would be something more along the lines of what was mentioned in the original message: use a rate.Limiter and dynamically adjust the rate based on the untruncated size of the Raft log. I foresee difficulties in achieving a stable rate limit with this approach.

[spencerkimball]

Yeah, I tried the rate.Limiter without getting any good results the last time I looked at this problem (when we were trying to handle slow recoveries pre-delete-range support). The biggest issue with it is setting the natural rate, which is definitely not obvious.

[petermattis]

I was thinking of adjusting the rate limit in raftLogQueue. That is, we would lower the rate limit as the un-truncated size of the Raft log grows and raise it when the Raft log is truncated. Might have to match this up with a moving average of the actual achieved rate over the previous few seconds.

[spencerkimball]

@petermattis ping.

[petermattis]

I'll clean up the existing PR this week.

[petermattis]

I never got this done due to other work taking precedence. Given that we don't have a known workload that would benefit from this, I'm moving to Later with the expectation that we'll address very early in the next release cycle.

[petermattis]

@irfansharif Is there anything left to do here given #15802?

[tbg]

For one, we have zero integration testing.

[irfansharif]

no but I want to run some more end-to-end tests demonstrating the behavior, was going to use this as an umbrella issue for that.

[bdarnell]

We've done more testing of this feature since it shipped in 1.1; I don't think there's anything else in the works for this issue.