tenantrate: relax host-side tenant rate limit #70328
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
RaduBerinde
force-pushed
the
relax-kv-side-throttling
branch
from
b34f94b to
35ee145
Compare
andy-kimball
approved these changes
Sep 17, 2021
There was a problem hiding this comment.
@darinpp, looks like Radu has got this change covered.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @ajwerner)
RaduBerinde
force-pushed
the
relax-kv-side-throttling
branch
from
35ee145 to
f681a55
Compare
Tenants will now be primarily throttled from the client side. This change increases the existing per-KV-node limit from an absolute 200 RU/s to 200 RU/s *per CPU*. This should roughly map to 20% of the capacity of a node. The reasoning is that we can limit the user-observed variability in performance if we prevent them from taking full advantage of an underloaded node. This also works as a mechanism to limit the possibility of a tenant single-handedly overloading a KV node. This increase is necessary to provide good performance to non-throttled (paying) tenants. We implement this change in a backward-compatible way by continuing to support setting absolute rates with `kv.tenant_rate_limiter.rate_limit`; we now use negative values to indicate per-CPU rates. Tested by checking for the log during TestTenantRateLimiter: ``` tenantrate limiter initialized (rate: 2400 RU/s; burst: 24000 RU) ``` Release note: None Release justification: Necessary fix to support paid Serverless tenants. This functionality is only enabled in multi-tenant scenarios and should have no impact on our dedicated customers.
RaduBerinde
force-pushed
the
relax-kv-side-throttling
branch
from
f681a55 to
4f77a8e
Compare
|
bors r+ |
15 tasks
|
Build succeeded: |
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this pull request
Sep 24, 2021
A recent change (cockroachdb#70328) increased the rate limit and that exposed a fragility in this test. This change restores the test to use the original (small) rate limit. Fixes cockroachdb#70456. Release note: None Release justification: test-only fix.
craig bot
pushed a commit
that referenced
this pull request
Sep 25, 2021
70723: kv: deflake TestTenantRateLimiter r=RaduBerinde a=RaduBerinde A recent change (#70328) increased the rate limit and that exposed a fragility in this test. This change restores the test to use the original (small) rate limit. Fixes #70456. Release note: None Release justification: test-only fix. Co-authored-by: Radu Berinde <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tenants will now be primarily throttled from the client side. This
change increases the existing per-KV-node limit from an absolute 200
RU/s to 200 RU/s per CPU. This should roughly map to 20% of the
capacity of a node. The reasoning is that we can limit the
user-observed variability in performance if we prevent them from
taking full advantage of an underloaded node. This also works as a
mechanism to limit the possibility of a tenant single-handedly
overloading a KV node.
This increase is necessary to provide good performance to
non-throttled (paying) tenants.
We implement this change in a backward-compatible way by continuing to
support setting absolute rates with
kv.tenant_rate_limiter.rate_limit; we now use negative values toindicate per-CPU rates.
Tested by checking for the log during TestTenantRateLimiter:
Release note: None
Release justification: Necessary fix to support paid Serverless
tenants. This functionality is only enabled in multi-tenant scenarios
and should have no impact on our dedicated customers.