server,kvserver: set tenant weights for admission control#77575
server,kvserver: set tenant weights for admission control#77575craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
ajwerner
left a comment
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)
sumeerbhola
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)
pkg/server/node_test.go, line 716 at r1 (raw file):
}) // Get weights and check. // TODO(sumeer): enhance test to have multiple tenants.
@ajwerner Any quick ideas on how to create ranges belonging to different tenants?
cucaroach
left a comment
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status:complete! 2 of 0 LGTMs obtained (waiting on @ajwerner and @cucaroach)
ajwerner
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 2 of 0 LGTMs obtained (waiting on @sumeerbhola)
pkg/server/node_test.go, line 716 at r1 (raw file):
Previously, sumeerbhola wrote…
@ajwerner Any quick ideas on how to create ranges belonging to different tenants?
You can do this:
const tenantID = 10
prefix := keys.MakeTenantPrefix(roachpb.MakeTenant(tenantID))
require.NoError(t, kvDB.AdminSplit(ctx, prefix, hlc.MaxTimestamp))
nvb
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 2 of 0 LGTMs obtained (waiting on @sumeerbhola)
pkg/util/admission/work_queue.go, line 223 at r1 (raw file):
// Note that currently there are no weights associated with tenants -- one // could imagine using weights and comparing used/weight values for // inter-tenant fairness.
Is it time to remove this comment?
nvb
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 2 of 0 LGTMs obtained (waiting on @sumeerbhola)
pkg/util/admission/work_queue.go, line 1124 at r1 (raw file):
ordered in increasing order of tenantInfo.used
Is this still accurate, as of 8f385b4?
The weights are used in ordering tenants, such that tenant i is preferred over tenant j if used_i/weight_i < used_j/weight_j, where the used values represent usage of slots or tokens. It allows for a primitive form of weighted fair sharing. This was added in a preceding PR. This PR integrates this existing support for weighting, by - making Node provide these weights by counting the number of ranges for each tenant, both per store and across the whole node. - making KVAdmissionControllerImpl periodically poll for these weights (at a 10min interval) and provide these to the single KV WorkQueue and each of the kv-stores WorkQueues. An alternative would be to put the polling logic inside the admission control package. The difficulty there is that we want to synchronize the computaton across the whole node, instead of doing it once per WorkQueue. KVAdmissionControllerImpl provides a convenient place to place this synchronized computation. The cluster settings for using weights are set to a default that disables weights. Informs cockroachdb#77358 Release justification: Low-risk update to new functionality. Even though inter-tenant isolation was included in v21.2, it has only been used in CockroachDB serverless recently, and there is consensus to include weighting for v22.1. Additionally, weights are disabled by default. Release note (ops change): The cluster settings admission.kv.tenant_weights.enabled and admission.kv.stores.tenant_weights.enabled can be used to enable tenant weights in multi-tenant storage servers. The default is disabled.
sumeerbhola
left a comment
There was a problem hiding this comment.
TFTRs!
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @ajwerner, @cucaroach, and @nvanbenschoten)
pkg/util/admission/work_queue.go, line 223 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Is it time to remove this comment?
ah, yes. Removed and adjusted the comment above.
pkg/util/admission/work_queue.go, line 1124 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
ordered in increasing order of tenantInfo.used
Is this still accurate, as of 8f385b4?
Good catch. Fixed.
pkg/server/node_test.go, line 716 at r1 (raw file):
Previously, ajwerner wrote…
You can do this:
const tenantID = 10 prefix := keys.MakeTenantPrefix(roachpb.MakeTenant(tenantID)) require.NoError(t, kvDB.AdminSplit(ctx, prefix, hlc.MaxTimestamp))
Thanks!
|
bors r=ajwerner,nvanbenschoten,cucaroach |
|
Build succeeded: |
The weights are used in ordering tenants, such that tenant i is
preferred over tenant j if used_i/weight_i < used_j/weight_j,
where the used values represent usage of slots or tokens. It
allows for a primitive form of weighted fair sharing. This
was added in a preceding PR.
This PR integrates this existing support for weighting, by
of ranges for each tenant, both per store and across the
whole node.
weights (at a 10min interval) and provide these to the
single KV WorkQueue and each of the kv-stores WorkQueues.
An alternative would be to put the polling logic inside the
admission control package. The difficulty there is that we
want to synchronize the computaton across the whole node,
instead of doing it once per WorkQueue.
KVAdmissionControllerImpl provides a convenient place to place
this synchronized computation.
The cluster settings for using weights are set to a default
that disables weights.
Informs #77358
Release justification: Low-risk update to new functionality.
Even though inter-tenant isolation was included in v21.2,
it has only been used in CockroachDB serverless recently,
and there is consensus to include weighting for v22.1.
Additionally, weights are disabled by default.
Release note (ops change): The cluster settings
admission.kv.tenant_weights.enabled and
admission.kv.stores.tenant_weights.enabled can be used to
enable tenant weights in multi-tenant storage servers. The
default is disabled.