Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Increase preparation timeout #4270

Merged
merged 3 commits into from
Nov 15, 2021
Merged

Increase preparation timeout #4270

merged 3 commits into from
Nov 15, 2021

Conversation

pepyakin
Copy link
Contributor

Lately we've been observing preparation timeouts. Whereas in local testing, compilation of a hefty PVF should take <1s for multi-threaded compilation (and ≈3s for single-threaded), in the wild we were observing compilation speed of more like 5 seconds and more occasionally bumping into the 10s cutoff.

This timeout affects the maximum response time for backing and approvals work. However, it is amortized, and once compiled it will be cached for time the artifact is being used. 10s cutoff was chosen rather arbitrary. It is already more than backing, which has way less than 10s, so it will be missed anyway. Missing the deadline once in a while for approvals does not seem bad either.

At the same time, increasing the timeout does not seem to be very bad at this moment. Exploiting JIT bombs does not seem to be likely. Doing so will require big investment and the effects of the attack are questionable.

Priority is set to high, because we need to upgrade the nodes ASAP to minimize the time when nodes have different opinion on whether the code timed out or not.

@pepyakin pepyakin added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C7-high ❗️ labels Nov 12, 2021
@pepyakin pepyakin added this to the v0.9.13 milestone Nov 12, 2021
@pepyakin pepyakin added B1-releasenotes and removed B0-silent Changes should not be mentioned in any release notes labels Nov 12, 2021
@ordian ordian merged commit 414ec0e into master Nov 15, 2021
@ordian ordian deleted the pep-bump-preparation-timeout branch November 15, 2021 11:48
drahnr pushed a commit that referenced this pull request Nov 15, 2021
* Increase preparation-timeout to 60 seconds

* Adapt `pvf_preparation_time` metric to the new value
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants