upgrade http4k, and fix database URL to have port, implement docker f…#3206
upgrade http4k, and fix database URL to have port, implement docker f…#3206NateBrady23 merged 20 commits intoTechEmpower:masterfrom http4k:master
Conversation
|
@daviddenton we're currently right in the middle of working on a way to integrate docker with the suite, but right now any tests using docker break our continuous benchmarking. See: #3187 |
|
The tests don’t use docker. There’s a just a Dockerfile and a script which would need to be manually run. Is that a problem? |
|
If it is a problem, feel free to ditch the file when you merge it in, or I can do it. But I’d like it not to block the merge :) |
|
Closing until R16, to ensure that we get results for R15. |
|
hey @daviddenton sorry, I didn't dig too deep there but I saw the dockerfile and a |
|
@daviddenton For what it's worth, I tried running this PR on Server Central and I still got 0 RPS for the database tests. I'm a little confused how it passes verification but has exactly 0 requests in the full test. |
|
I’m presuming that the change is good though for that environment? It does need the port to connect - or is there something unusual about that env? |
|
And obviously it does work with the dockerised env that I included in the dockerfile. :) |
|
Could it be the benchmark JSON - is the name of the database correct? Not sure if it’s case sensitive.. EDIT: Following the above suspicion, I've updated the name of the database - if anyone would like to test it again against SC that'd be awesome. |
…red upgrade to http4k
|
@daviddenton It's like it stops responding to requests after wrk starts the load test. It's ok at first, and it returns the right content from all the db-related endpoints so we know it's reaching the database, but then it falls over when wrk starts making a ton of requests. I got it to respond with an error page when did a curl while the load test was running: |
|
@michaelhixson found the problem - it was that we were not closing off resources properly, so it basically eats up all the connections and then chokes. Thanks a miiion for your help in getting this resolved. 👏 ⭐️ 😄 I'll reopen this as soon as R15 is done. :) |
|
Reopening to get into R16 previews. Keep up the good work people! 😄 |
|
Hi @daviddenton can you check out #3308 just make sure the benchmark_config.json structure is correct. Also, could you add the framework to |
|
@nbrady-techempower yep - that all looks good. Thanks for fixing that JSON thing - I always wondered why we got that warning and the split of the tests JSON array was too subtle for me to spot. On that subject, I'd actually like to ditch the "default" completely, since we don't really have a "default" - we have several pluggable backends (currently the default is actually the jetty implementation). Is this possible? If it isn't, then please forget I said anything and just merge this :) |
|
@daviddenton The Now that travis is picking up the test, it looks like it's not passing there. |
|
@daviddenton Could you remove the local testing scripts like |
* Delete benchmark_all.sh * Delete benchmark_server.sh * Delete killbenchmark.sh * Delete run_db.sh * Delete Dockerfile
|
@michaelhixson no problem. tidied as requested. |
…or postgres
If we could get this into final R15, that'd be awesome as it fixes the long broken DB tests