Skip to content
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

ci: loadtest PRs against latest releases #2960

Merged

Conversation

develop7
Copy link
Collaborator

@develop7 develop7 commented Sep 20, 2023

Lets postgrest-loadtest-against accept multiple refs performing loadtests against all of them; and
introduces loadtesting PRS against latest release as well.

The downside is we're now unable to pass addtional parameters to vegeta (due to limitation of argbash's support of infinite parameters), please let me know if this is a problem (/cc um… @wolfgangwalther?)

Improve postgrest-loadtest-against script to accept multiple refs
to perform loadtests against *all* of them
Leveraging the previously-improved postgrest-loadtest-against
introduces loadtesting PRS against latest release as well.
@steve-chavez
Copy link
Member

steve-chavez commented Sep 20, 2023

Interesting! The load test result: https://github.com/PostgREST/postgrest/pull/2960/checks?check_run_id=16970655907

Another data point will also help clear some random noise, as it was the case on #2934 (comment)

The downside is we're now unable to pass addtional parameters to vegeta

I'd be fine with that but let's see what Wolfgang thinks.

@wolfgangwalther
Copy link
Member

The downside is we're now unable to pass addtional parameters to vegeta (due to limitation of argbash's support of infinite parameters), please let me know if this is a problem

No problem for me. I have not used those additional parameters, so far. It just seemed nice to implement them... because we were able to do so. Now we have a better way of using those arguments: 👍

Another data point will also help clear some random noise

The loadtest job is still running only 3 min. To clear up the random noise it could also be helpful to do something like:

  • head
  • main
  • head
  • latest tag
  • head

This would still just take 5 min, but it would eliminate more of the "we had more/less load on the server in the beginning of the run compared to in the end".

@develop7
Copy link
Collaborator Author

@wolfgangwalther from what I've gathered during the load testing in CI is the impression that the resources budget allocated to the VM the CI job runs in is probably consistent across VM's lifetime. OTOH, I agree that multiple runs would let us know how consistently is postgrest performing under the load. Shouldn't be hard to implement, will do with copy&paste for clarity's sake.

And since we're having multiple runs, I'd suggest adding some extra stats, i.e. mean & sigma and/or min & max. Or, even better, since there's not very much runs against the same revision, add delta against mean value for each run. Of course this one seems to deserve for separate issue at least.

@steve-chavez steve-chavez merged commit add4dfe into PostgREST:main Sep 21, 2023
30 checks passed
@develop7 develop7 deleted the develop7/feat-loadtest_prs_release branch September 22, 2023 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants