- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 60
add optional concurrent max queue limit: #2940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- set 'max_concur_queue_to_limit_scale' to determine max size of concurrent crawls that can be queued - if set, above limit queueing new crawls will be rejected with a 429 - default to disabled tests: add max queue limit to concurrent crawl tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general working well! Tested locally with a limit scale multiplier of 1 and 2.
The nightly concurrent crawl limit test doesn't pass yet, should be tweaked a bit. I left a comment in-line.
I wonder if we also want to have a limit for the number of queued crawls even when a concurrent crawl quota isn't set? Not sure if self-deployments that don't use a concurrent crawl limit might still run into resourcing issues from starting too many crawls at the same time.
Only other comment is that I find max_concur_queue_to_limit_scale to be pretty cryptic. Without reading the comments in values.yaml I'm not sure I'd be able to piece together what it means. Something like crawl_queue_limit_scale might be a bit easier to read?
| 
 PR #2945 adds a separate optimization which should make the concurrent crawl check more efficient in general, even if there is no limit. I think that should avoid the main resourcing issue, and maybe makes this PR is less important, but still an option to have. | 
Co-authored-by: Tessa Walsh <[email protected]>
Testing:
max_concur_queue_to_limit_scale: 1