Skip to content
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

feat(router): limit number of jobs with the same ordering key in worker's queue #3243

Merged
merged 2 commits into from
May 3, 2023

Conversation

atzoum
Copy link
Contributor

@atzoum atzoum commented Apr 25, 2023

Description

Until now, a single concurrency limiter existed in router's barrier, i.e. our event ordering mechanism, which was there to ensure that when a failed job got drained (aborted), in the next router generator loop only a single job would be picked up by the router for the same ordering key. This happens so as not to flood the worker's queue with jobs which are most probably going to be marked as waiting later on.

We are now introducing an additional concurrency limiter, which is responsible for limiting the overall maximum number of jobs that can enter the barrier for the same ordering key, regardless of failures. This barrier will protect us from a single "chatty" ordering key saturating the worker’s queue, so that jobs for other ordering keys can have a fair chance of being picked up by router.

The new concurrency limiter can be disabled by setting the limit to 0.

Notion Ticket

Link

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Patch coverage: 93.75% and project coverage change: +0.06 🎉

Comparison is base (be1fef9) 51.85% compared to head (0886083) 51.91%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3243      +/-   ##
==========================================
+ Coverage   51.85%   51.91%   +0.06%     
==========================================
  Files         322      322              
  Lines       53381    53411      +30     
==========================================
+ Hits        27682    27730      +48     
+ Misses      24042    24028      -14     
+ Partials     1657     1653       -4     
Impacted Files Coverage Δ
router/internal/eventorder/eventorder.go 96.13% <92.68%> (-0.30%) ⬇️
router/router.go 76.12% <100.00%> (+0.08%) ⬆️
...-forwarder/internal/forwarder/abortingforwarder.go 74.50% <100.00%> (ø)
warehouse/internal/repo/staging.go 81.11% <100.00%> (+0.18%) ⬆️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@atzoum atzoum changed the title [WIP] chore: limit the maximum number of jobs with the same ordering key in router [WIP] chore: limit the maximum number of jobs with the same ordering key in router's worker queue Apr 26, 2023
@atzoum atzoum force-pushed the chore.globalRouterLimiter branch 3 times, most recently from a4c3117 to 3769417 Compare April 28, 2023 06:10
@atzoum atzoum changed the title [WIP] chore: limit the maximum number of jobs with the same ordering key in router's worker queue feat: limit the maximum number of jobs with the same ordering key in router's worker queue Apr 28, 2023
@atzoum atzoum changed the title feat: limit the maximum number of jobs with the same ordering key in router's worker queue feat(router): limit the maximum number of jobs with the same ordering key in worker's queue Apr 28, 2023
@atzoum atzoum changed the title feat(router): limit the maximum number of jobs with the same ordering key in worker's queue feat(router): limit number of jobs with the same ordering key in worker's queue Apr 28, 2023
@atzoum atzoum requested a review from achettyiitr April 28, 2023 07:13
@atzoum atzoum force-pushed the chore.globalRouterLimiter branch from 57d1b78 to 0886083 Compare May 2, 2023 11:55
@atzoum atzoum requested a review from fracasula May 2, 2023 11:55
@atzoum atzoum merged commit ed5722d into master May 3, 2023
17 checks passed
@atzoum atzoum deleted the chore.globalRouterLimiter branch May 3, 2023 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants