paritytech / bench-bot

ISC License
9 stars 19 forks source link

Upload the weight's diff if it fails to push for some reason #54

Open joao-paulo-parity opened 3 years ago

joao-paulo-parity commented 3 years ago

At the moment, in case some error occurs while the bot is trying to push the generated weights (e.g. https://github.com/paritytech/substrate/pull/9686#issuecomment-915911689), the bot will simply error. It would be better to upload the weight's diff that the effort isn't lost (some benchmarks take a long time to run).

Github limits the amount of characters per-comment so that's something to also take into account before posting the diff. In case the maximum length is exceeded, the diff could be uploaded as a Gist.

athei commented 2 years ago

You could use git format-patch to create a patch file and compress this and use a file upload instead of posting it as text. This would circumvent the limitation and also makes it easier to apply.

shawntabrizi commented 2 years ago

CC @ggwpez

We were already thinking of generating raw JSON output files from all the benchmarks so that we could generate visual representations of the benchmarks in case people want to look closer at the results.

From this, we should also be able to easily generate the weight files if they could not be committed.

ggwpez commented 2 years ago

So where do we upload it to? I don't think creating Gists is optimal.
Maybe some S3 compatible server to throw the JSON files in?

shawntabrizi commented 2 years ago

Can we not just leave it on the benchmarking machine?

I guess we would be concerned about serving files while it was benchmarking?

If we need another server that works and can be allocated, otherwise, gist is probably not the worst idea.

ggwpez commented 2 years ago

Can we not just leave it on the benchmarking machine? I guess we would be concerned about serving files while it was benchmarking?

Yes, just running a webserver there could interfere with the results.
In the best case we would make that data publicly available, so that other people can also analyze Substrate+Polkadot weights.

If we need another server that works and can be allocated, otherwise, gist is probably not the worst idea.

If we just throw them in single Gist files we will not be able to index/order or find them ever again.
To not go overboard for the first version of this; some kind of indexable file storage e.g. S3-compatible Bucket storage would be fine I think.

joao-paulo-parity commented 2 years ago

I'm assigning myself to this task since I plan to work on it sometime soon

athei commented 2 years ago

So where do we upload it to? I don't think creating Gists is optimal. Maybe some S3 compatible server to throw the JSON files in?

I attachments to postings I make in github by dragging them in here and it will just upload them "somewhere". Can't the bot do the same thing?

ggwpez commented 2 years ago

I attachments to postings I make in github by dragging them in here and it will just upload them "somewhere". Can't the bot do the same thing?

Yes for the diff that is enough. Sorry for mixing up two issues, my concern was regarding the raw output.

koenw commented 2 years ago

If we just throw them in single Gist files we will not be able to index/order or find them ever again.

We would need to keep an index of Gists, perhaps in a file (or files) that we can commit/push.

Is there any other downside to Gists? It seems simpler to me than using an additional service.

We would also easily be able to generate (or it could just be) a JSON of all benchmarks like @shawntabrizi mentioned.

joao-paulo-parity commented 2 years ago

Recent incidents required manual intervention: