kvserver: use MaxInflightBytes raft config option

[pav-kv]

This change starts using the raft's MaxInflightBytes option which limits the inflight total byte size of the log entries sent via MsgApp. The default limit is set to a conservatively large value, to not change the current behaviour for now.

Part of #90314
Epic: none
Release note (ops change): Added COCKROACH_RAFT_MAX_INFLIGHT_BYTES env variable which helps strictly limiting inflight traffic from a Raft leader to followers, particularly in situations when many large messages are sent and significantly exceed COCKROACH_RAFT_MAX_SIZE_PER_MSG * COCKROACH_RAFT_MAX_INFLIGHT_MSGS which is a softer limit.

[cockroach-teamcity]

This change is 

[erikgrinaker]

LGTM

[tbg]

I wonder if the default here should not be hard-coded, but constructed dynamically based on MaxInflightMsgs and MaxMsgSize. Something like:

maxInflightBytes := 256 << 20
if other := maxBytesPerMsg * maxInflightMsgs {
  maxInflightBytes = other
}

We have some customers that configure MaxInflightMsgs to very high values. Presumably they'd still end up below 256<<20 (in actual BDP) for the maxInflightBytes but it does make some sense for the maxInflightBytes to always be larger than the bandwidth-delay product implied by the other two settings, unless it's explicitly specified.

[pav-kv]

Added a fixup commit (not merging yet, testing the idea). Does this look any reasonable?

[pav-kv]

The idea is: if either RaftConfig or env variable has this option non-zero, then the user knows what they are doing, so we don't check. Otherwise we compute the default which is a multiple of the other 2 parameters (which can either be default too, or overridden by the user).

Alternatively, we could by default set MaxInflightBytes to 0, which would disable this feature. But then we would need to explicitly modify our prod environment to enable it. Which of the ways is better?

[pav-kv]

Ended up using your suggestion.

I think RaftMaxInflightBytes is an important limit on its own, it shouldn't heavily depend on RaftMaxBytesPerMsg because the latter is too much of a soft limit. So I reverted my idea of dynamically setting it to maxBytesPerMsg * maxInflightMsgs * 64 because that presumes that the largest message shouldn't normally exceed 64x target message size. I think that the users concerned with the bandwidth-delay product should primarily be tweaking RaftMaxInflightBytes.

Setting the default RaftMaxInflightBytes to a constant allows tweaking the other 2 parameters independently to optimize for different things. Since maxBytesPerMsg is effectively a "target" message size, it controls the batching strategy, and not so much the max-inflight traffic. Another interpretation of maxBytesPerMsg is: it's the subjective threshold between "small" and "large" messages. Which is also why I don't think it should determine the max-inflight-bytes.

Upping RaftMaxInflightBytes to the product makes sense though, to not accidentally break users who already assume things based on the other 2 settings. Alternatively, we could panic if the config is not reasonable? But I don't know if that would break any existing users.

[tbg]

Basically LGTM but need to fix overflow.

[pav-kv]

bors r=erikgrinaker,tbg

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)