-
Notifications
You must be signed in to change notification settings - Fork 311
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
chore: drop union query, fair pickup and multitenant handle #3521
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3521 +/- ##
==========================================
+ Coverage 67.79% 67.81% +0.02%
==========================================
Files 325 327 +2
Lines 52627 53100 +473
==========================================
+ Hits 35678 36012 +334
- Misses 14603 14707 +104
- Partials 2346 2381 +35
☔ View full report in Codecov by Sentry. |
2c2236b
to
a5aafc6
Compare
e97bfce
to
a0d59df
Compare
unprocessedAfterJobID := unprocessed.Jobs[len(unprocessed.Jobs)-1].JobID | ||
mtoken.unprocessedAfterJobID = &unprocessedAfterJobID | ||
} | ||
list = append(list, unprocessed.Jobs...) |
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.
[question] does it make sense to move this in the previous if block? what purpose does it serve if len(unprocessed.Jobs) == 0
?
waitingAfterJobID := waiting.Jobs[len(waiting.Jobs)-1].JobID | ||
mtoken.waitingAfterJobID = &waitingAfterJobID | ||
} | ||
list = append(list, waiting.Jobs...) |
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.
[question] does it make sense to move this in the previous if block? what purpose does it serve if len(waiting.Jobs) == 0
?
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.
if waiting.Jobs is empty this is a no-op, so it doesn't hurt either :)
a0c9e28
to
b30f881
Compare
b30f881
to
fd0963d
Compare
Description
Since router isolation feature is now in place, there is no longer the need for the fair pickup algorithm, i.e.
MultiTenantJobsDB
andmultitenant.Stats
. Therefore we are removing them with the following adaptations:MultiTenantJobsDB#GetAllJobs
has been moved toJobsDB#GetToProcess
and method's signature has been adapted accordingly. A future improvement shall follow which will adapt the method's current implementation and perform a single query instead of 3 separate ones.processor
is now responsible for retrieving job pileup counters and populating the pending events stats during startup, what was previously done bymultitenant.Stats
Notion Ticket
Link
Security