Feature/postgres 13 theory #4396
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4396 +/- ##
==========================================
Coverage 69.23% 69.23%
Complexity 3691 3691
==========================================
Files 267 267
Lines 15197 15197
Branches 1651 1651
==========================================
Hits 10522 10522
Misses 3907 3907
Partials 768 768
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
| @@ -42,7 +42,7 @@ services: | |||
| # networks: | |||
| # - elastic | |||
| postgres_db: | |||
| image: postgres:11.12 | |||
| image: postgres:13.3 | |||
There was a problem hiding this comment.
Add command: postgres -c max_connections=200 -c jit=off here as well? Although maybe the max_connections portion isn't necessary.
There was a problem hiding this comment.
for JIT see other pg 12 PR.
for 200 connections, the comment is originally
Some sort of connection leak (by the internal dropwizard validation query), could not figure out how to fix, just increased db max_connections
in relation to I think tests, so I think it doesn't apply here @garyluu can you confirm?
There was a problem hiding this comment.
for JIT see other pg 12 PR.
Do you mean other PR turns off JIT, or there's a comment there I should look at?
There was a problem hiding this comment.
I think I meant this comment #4390 (comment) which was made when I assumed AWS RDS had JIT on. In that case, I didn't decide whether I wanted JIT on or off for Docker compose to match the tests or match prod.
Since you discovered that prod has JIT off, think I'll turn it off here too
| @@ -178,7 +179,7 @@ public void testProprietaryAPI() { | |||
| * @param path Path of endpoint | |||
| */ | |||
| private void testXTotalCount(Client jerseyClient, String path) { | |||
| Response response = jerseyClient.target(path).request().get(); | |||
| Response response = jerseyClient.target(path).request().property(ClientProperties.READ_TIMEOUT, 0).get(); | |||
There was a problem hiding this comment.
Just curious, why did this need to change?
There was a problem hiding this comment.
Without it, one of the tests can timeout.
It is possible the slower DB was responsible for slowing things to the extent it timed out, but I figure it couldn't hurt to keep it.
| @@ -42,7 +42,7 @@ services: | |||
| # networks: | |||
| # - elastic | |||
| postgres_db: | |||
| image: postgres:11.12 | |||
| image: postgres:13.3 | |||
There was a problem hiding this comment.
for JIT see other pg 12 PR.
Do you mean other PR turns off JIT, or there's a comment there I should look at?
|
Kudos, SonarCloud Quality Gate passed!
|
slightly slower performance than with 12.7, but is in the right neighbourhood
companion for #4388
https://app.slack.com/archives/CEGMXHBHT/p1628280502020200 for more details