Skip to content

Commit

Permalink
kvflowcontrol: use 'apply_to_elastic' mode by default
Browse files Browse the repository at this point in the history
We've been exercising the flow control machinery in the 'apply_to_all'
mode for the last few months, to shake out latent bugs and surface
performance regressions. Since flow control shapes quorum writes to the
rate of IO admission by the slowest live replica, it's not
necessarily the scheme we want by default for latency-sensitive
foreground writes. It does however make sense for elastic writes, which
by definition is not latency sensitive, and write shaping is ok.

With 'apply_to_elastic', regular writes are still subject to admission
control. But it happens only on the leaseholder where the write
originates. On nodes observing follower writes from regular work, we'll
deduct IO tokens without waiting. Crucially, with 'apply_to_elastic',
this deduction-without-waiting will not happen for elastic writes, which
tend to be bulkier and by definition, lower priority than regular
writes.

It might still make sense for users to opt into 'apply_to_all'. Either
in this universal cluster setting form, or perhaps when exposed more
selectively through zone configs, so it's done on a schema-by-schema
basis. Consider setups with heterogenous regions where nodes in
particular regions only contain follower replicas and also have lower IO
admission rates (by virtue of there being fewer nodes in aggregate, or
with hardware provisioned with lesser throughput). Without any write
shaping, if the leader-only regions drive at higher throughput, the
follower region will fall permanently behind, or will have uncontrolled
LSM growth, making it inappropriate for fail overs. In such cases, users
may want to in fact shape their quorum writes based on the IO admission
rates of the slowest follower replica.

Release note: None
  • Loading branch information
irfansharif committed Sep 5, 2023
1 parent d9c137d commit fb5bd27
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 1 deletion.
1 change: 1 addition & 0 deletions pkg/kv/kvserver/flow_control_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,7 @@ func TestFlowControlRaftSnapshot(t *testing.T) {
ctx := context.Background()
st := cluster.MakeTestingClusterSettings()
kvflowcontrol.Enabled.Override(ctx, &st.SV, true)
kvflowcontrol.Mode.Override(ctx, &st.SV, int64(kvflowcontrol.ApplyToAll))

for i := 0; i < numServers; i++ {
stickyServerArgs[i] = base.TestServerArgs{
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/kvflowcontrol/kvflowcontrol.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ var Mode = settings.RegisterEnumSetting(
settings.SystemOnly,
"kvadmission.flow_control.mode",
"determines the 'mode' of flow control we use for replication traffic in KV, if enabled",
ApplyToAll.String(),
ApplyToElastic.String(),
map[int64]string{
int64(ApplyToElastic): modeDict[ApplyToElastic],
int64(ApplyToAll): modeDict[ApplyToAll],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ func TestInspectController(t *testing.T) {
st := cluster.MakeTestingClusterSettings()
elasticTokensPerStream.Override(ctx, &st.SV, 8<<20 /* 8 MiB */)
regularTokensPerStream.Override(ctx, &st.SV, 16<<20 /* 16 MiB */)
kvflowcontrol.Mode.Override(ctx, &st.SV, int64(kvflowcontrol.ApplyToAll))
controller := New(metric.NewRegistry(), st, hlc.NewClockForTesting(nil))

// No streams connected -- inspect state should be empty.
Expand Down

0 comments on commit fb5bd27

Please sign in to comment.