Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

Perform performance degradation check before deploying #48

Open
joao-paulo-parity opened this issue Jul 14, 2021 · 3 comments
Open

Perform performance degradation check before deploying #48

joao-paulo-parity opened this issue Jul 14, 2021 · 3 comments

Comments

@joao-paulo-parity
Copy link
Contributor

joao-paulo-parity commented Jul 14, 2021

We should have a workflow in the deploy process where before deploying a new version

  1. Revert the bot's code to a known good commit
  2. Revert the Substrate to a known good commit
  3. Run a benchmark
  4. Update the bot's code
  5. Run the benchmark again
  6. Compare the results

This would avoid confusion such as the one seen in #47 because we'd know upfront which commit introduced a regression when the bot is updated.

It's relevant for this workflow to be executed directly in the machine, rather than e.g. CI, so that the results can be compared more predictably.

@joao-paulo-parity
Copy link
Contributor Author

This would be useful for #49 since we don't want to go through #47 again.

At the moment this "sanity degradation check" will have to be performed manually.

@joao-paulo-parity joao-paulo-parity changed the title Sanity degradation check pre-deploy Peform performance degradation check before deploying Sep 1, 2021
@joao-paulo-parity joao-paulo-parity changed the title Peform performance degradation check before deploying Perform performance degradation check before deploying Sep 1, 2021
@ggwpez
Copy link
Member

ggwpez commented Jan 19, 2022

I like the idea, it's similar to #56.
Instead of running the benchmark twice, the previous run can be saved into a file when executed with --raw.
Making these files available would be a next big step and would allow to compare performance across commits.

There is this #51 refactor issue open. Do you think this should be done first?
@joao-paulo-parity

@joao-paulo-parity
Copy link
Contributor Author

There is this #51 refactor issue open. Do you think this should be done first?

I consider #68 to be the biggest problem of processbot right now. That is what I would work on if I were available to work on this project. If #68 can't be figured out because of Node.js, then the suggestion of #51 (comment) incurs a rewrite (in another language) rather than a refactor. In any case, I would not work on any new feature, including this one, until #68 is dealt with.

My personal priorities are not a blocker for the project and developers are free to work on whatever they find most relevant. If it were up to me then, to answer your question, no, I would not work on this first because I would prioritize something else. If another developer wants to work on this ticket, or any other for that matter, I don't see a problem with that.

@Vovke Vovke added duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists labels Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants