fix(update_branches): commit more often, no more yield_per, no more ownerid #257
Conversation
joseph-sentry
commented
Feb 7, 2024
•
joseph-sentry
commented
- no more ownerid
- takes in incorrect commitid to help filter branches
- no longer uses yield_per, instead does one query for all the branches then breaks it up into chunks of branches
- commits after updating every chunk
Codecov ReportAttention: @@ Coverage Diff @@
## main #257 +/- ##
=======================================
Coverage 98.11% 98.12%
=======================================
Files 377 377
Lines 31462 31460 -2
=======================================
+ Hits 30868 30869 +1
+ Misses 594 591 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #257 +/- ##
=======================================
Coverage 98.11% 98.12%
=======================================
Files 377 377
Lines 31462 31460 -2
=======================================
+ Hits 30868 30869 +1
+ Misses 594 591 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention:
@@ Coverage Diff @@
## main #257 +/- ##
=======================================
Coverage 98.11% 98.12%
=======================================
Files 377 377
Lines 31462 31460 -2
=======================================
+ Hits 30868 30869 +1
+ Misses 594 591 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
tasks/update_branches.py
Outdated
| ): | ||
| if branch_name is None: | ||
| log.warning("No branch name specified, not updating any branches") | ||
| return {"attempted": False} | ||
|
|
||
| log.info( | ||
| "Doing update branches for branch", | ||
| extra=dict(branch_name=branch_name, ownerid=ownerid), | ||
| extra=dict(branch_name=branch_name), |
There was a problem hiding this comment.
[nit] I'd put the incorrect_commit here too since it changes the filter
tasks/update_branches.py
Outdated
| ) | ||
| if existing_commit is not None: | ||
| latest_commit_on_branch = ( | ||
| db_session.query(Commit) |
There was a problem hiding this comment.
This query will run many times.
If we want to optimize to run the least number of queries in the DB as possible (as to now disturb normal operations) I think this should be refactored.
The trade-off as you'd expect is use more memory in the task to pre-compute certain values.
My recommendation, if possible (haven't looked into it but I think it should be) is to get the latest commit for all the repoid in the chunk once before the start of the loop and use that info to make the updates.
something like
SELECT Commit WHERE branch == branch_name, repoid IN (repoids) GROUP BY repoid -picking last row by updatetimestamp-
The part I don't know exactly how to do is the -picking last row by updatetimestamp- but I think it should be possible to do that...
What do you think about that? Assuming to make the SELECT you need to possibly wait for locks maybe it's faster in the aggregate to do this.
There was a problem hiding this comment.
that makes a lot sense, I didn't think I could optimize this part but it makes sense to try to only do one query, I will look into this
joseph-sentry
force-pushed
the
joseph/improve-branch-head-script
branch
2 times, most recently
from
1f8d7c9 to
3c12bf7
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #257 +/- ##
=======================================
Coverage 98.08% 98.09%
=======================================
Files 408 408
Lines 32163 32161 -2
=======================================
+ Hits 31548 31549 +1
+ Misses 615 612 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
This change has been scanned for critical changes. Learn more |
…wnerid Signed-off-by: joseph-sentry <[email protected]>
Signed-off-by: joseph-sentry <[email protected]>
Signed-off-by: joseph-sentry <[email protected]>
joseph-sentry
force-pushed
the
joseph/improve-branch-head-script
branch
from
3c12bf7 to
a254743
Compare
