storage: make queue timeouts controllable, snapshot sending queues dynamic

[ajwerner]

In #42686 we made the raft snapshot queue timeout dynamic and based on the
size of the snapshot being sent. We also added an escape hatch to control
the timeout of processing of that queue. This change generalizes that
cluster setting to apply to all of the queues.

It so happens that the replicate queue and the merge queue also sometimes
need to send snapshots. This PR gives them similar treatment to the
raft snapshot queue.

The previous cluster setting was never released and is reserved so it does not
need a release note.

Release note: None

[cockroach-teamcity]

This change is 

[ajwerner]

@knz this relates to bad behavior we observed in a cluster this week. I'd like to backport both this and #42686 to 19.1 and 19.2.

[ajwerner]

Flaked on #44789

[knz]

Minor nit on terminology. Code LGTM otherwise.

What's our testing story around this code? I am surprised to see no test.

Reviewed 7 of 7 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)

---

pkg/storage/merge_queue.go, line 106 at r1 (raw file):

			// sometimes the merge queue needs to send multiple snapshots so perhaps
			// it would want even more time than the snapshot queue. That being said
			// this timeout give leeway for snapshots to be 10x slower than the

This sentence is either too long or ungrammatical, I don't understand it.

---

pkg/storage/queue.go, line 52 at r1 (raw file):

// queue will time out.
var queueMinimumTimeout = settings.RegisterDurationSetting(
	// NB: this setting has a relatively awkward name because the linter does not

Neither timeout_minimum nor minimum_timeout are good names. The linter is trying to tell you something. :)

Your description says it - guaranteed latency budget (time allotment without a timeout): kv.queue.process.guaranteed_latency_budget.

(English semantics: generally the word "minimum" is not clear if you don't specify what it is the minimum of - there needs to be multiple values to start with.)

[ajwerner]

Thanks for making write the tests. It needed them. Added a second commit to ensure that our rate settings are always positive. We would have had divide by zero panics both in this code and in the code that used them if they were set to 0.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz)

---

pkg/storage/merge_queue.go, line 106 at r1 (raw file):

Previously, knz (kena) wrote…

This sentence is either too long or ungrammatical, I don't understand it.

I reworked this comment.

---

pkg/storage/queue.go, line 52 at r1 (raw file):

Previously, knz (kena) wrote…

Neither timeout_minimum nor minimum_timeout are good names. The linter is trying to tell you something. :)

Your description says it - guaranteed latency budget (time allotment without a timeout): kv.queue.process.guaranteed_latency_budget.

(English semantics: generally the word "minimum" is not clear if you don't specify what it is the minimum of - there needs to be multiple values to start with.)

See how you feel about this.

[ajwerner]

@knz friendly ping

[knz]

Reviewed 2 of 9 files at r4, 7 of 7 files at r5.
Reviewable status: complete! 0 of 0 LGTMs obtained

[ajwerner]

TFTR!

bors r=knz

[craig]

Build succeeded

• GitHub CI (Cockroach)