feat: add more rate limit control flags #5570
Conversation
There was a problem hiding this comment.
LGTM. Related, but semi off-topic: Should coder scaletest recommend people set --dangerous-disable-rate-limits if there is a rate limit error, or should we recommend most people scale test with concurrency=1? I ran into rate limits at the 60 user mark.
Regardless, I'll certainly document the flag in the scaling docs. The doc will also recommend scale testing on a staging deployment versus the prod one (where it would be dangerous to disable all rate limits on all occasions)
|
Rate limits at low concurrency don't produce much load so I don't think we should recommend that. I think recommending this flag + using a staging deployment is OK. If customers really want to test on prod they'll most likely have to disable rate limits temporarily while they run their tests. |
|
Can we just add a single disable rate limit flag? Three seems excessive for this narrow use case. |
|
We already have a flag to configure the API rate limit, so I think we should expose flags for configuring the other rate limits as well. Right now they are magic numbers in the code. Edit: I've removed the new flags in favor of just the disable flag. |
Adds
--dangerous-disable-rate-limitsflag to entirely disable rate limits globally.Also:
DeploymentConfigField[].EnvOverridebecause I moved the value in the deployment config struct and the name got changed.str.replaceAlltostr.replaceto fix node version differences causing swagger generation to fail in dogfoodCloses #5531