Closed tfg13 closed 3 years ago
Merging #267 (fe2f623) into master (307dfa0) will increase coverage by
0.00%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #267 +/- ##
=======================================
Coverage 66.45% 66.45%
=======================================
Files 125 125
Lines 5959 5971 +12
=======================================
+ Hits 3960 3968 +8
Misses 1531 1531
- Partials 468 472 +4
Flag | Coverage Δ | |
---|---|---|
e2e | 45.30% <100.00%> (+0.01%) |
:arrow_up: |
integration | 59.90% <100.00%> (-0.02%) |
:arrow_down: |
unittests | 43.48% <100.00%> (+0.08%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
pkg/jobmanager/jobmanager.go | 78.19% <100.00%> (+1.03%) |
:arrow_up: |
pkg/jobmanager/status.go | 60.46% <100.00%> (ø) |
|
pkg/runner/job_runner.go | 64.50% <100.00%> (ø) |
|
pkg/cerrors/cerrors.go | 0.00% <0.00%> (-16.67%) |
:arrow_down: |
pkg/transport/http/http.go | 48.95% <0.00%> (-3.27%) |
:arrow_down: |
pkg/runner/test_runner.go | 93.19% <0.00%> (+0.70%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 307dfa0...fe2f623. Read the comment docs.
Status
processing in the server mainloop no longer blocks the mainloop. This is important since Status performs a bunch of DB queries that can take a while (depending on DB latency). All other calls are still sync, which makes reasoning about things like job ID allocation easier (they are also fast anyway).*Async
named query functions that can (but don't have to) be backed by a separate database connection to a read-only replica (to reduce load on the primary instance and spread DB queries). One of the query function in Status was not using the ro backend, leading to potential writes in this codepath.Note: There were complaints before about naming these functions
*Async
, suggestions for better names are still accepted :)Test Plan:
I put a sleep for 5s in status.go, then ran something like
go run [...]contestcli status 1 & go run [...]contestcli status 1 & go run [...]contestcli status 1 & go run [...]contestcli status 1
. This means the client is running in parallel, and server processing may or may not.Before the change:
0.81s user 1.23s system 9% cpu 21.154 total
After the change:
0.84s user 1.38s system 32% cpu 6.771 total