Changing PVF execution environment parameters only affecting execution shouldn't trigger artifact re-compilation #4132
Assignees
Comments
|
Also, if we need to change them, we should announce that this is happening. Let's add documentation explaining the effects on parachain block times and finality. |
|
This issue has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/async-backing-development-updates/6176/9 |
3 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Apr 25, 2024
Currently, PVFs are re-prepared if any execution environment parameter changes. As we've recently seen on Kusama and Polkadot, that may lead to a severe finality lag because every validator has to re-prepare every PVF. That cannot be avoided altogether; however, we could cease re-preparing PVFs when a change in the execution environment can't lead to a change in the artifact itself. For example, it's clear that changing the execution timeout cannot affect the artifact. In this PR, I'm introducing a separate hash for the subset of execution environment parameters that changes only if a preparation-related parameter changes. It introduces some minor code duplication, but without that, the scope of changes would be much bigger. TODO: - [x] Add a test to ensure the artifact is not re-prepared if non-preparation-related parameter is changed - [x] Add a test to ensure the artifact is re-prepared if a preparation-related parameter is changed - [x] Add comments, warnings, and, possibly, a test to ensure a new parameter ever added to the executor environment parameters will be evaluated by the author of changes with respect to its artifact preparation impact and added to the new hash preimage if needed. Closes #4132
github-project-automation
bot
moved this from Backlog
to Completed
in parachains team board
TarekkMA
pushed a commit
to moonbeam-foundation/polkadot-sdk
that referenced
this issue
Aug 2, 2024
Currently, PVFs are re-prepared if any execution environment parameter changes. As we've recently seen on Kusama and Polkadot, that may lead to a severe finality lag because every validator has to re-prepare every PVF. That cannot be avoided altogether; however, we could cease re-preparing PVFs when a change in the execution environment can't lead to a change in the artifact itself. For example, it's clear that changing the execution timeout cannot affect the artifact. In this PR, I'm introducing a separate hash for the subset of execution environment parameters that changes only if a preparation-related parameter changes. It introduces some minor code duplication, but without that, the scope of changes would be much bigger. TODO: - [x] Add a test to ensure the artifact is not re-prepared if non-preparation-related parameter is changed - [x] Add a test to ensure the artifact is re-prepared if a preparation-related parameter is changed - [x] Add comments, warnings, and, possibly, a test to ensure a new parameter ever added to the executor environment parameters will be evaluated by the author of changes with respect to its artifact preparation impact and added to the new hash preimage if needed. Closes paritytech#4132
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Changing parameters like max. memory pages or execution timeouts has nothing to do with the artifact preparation but still causes re-compilation of every single artifact, resulting in liveness issues on production networks. The parameters should be split into groups that may intersect and only trigger the re-compilation if a preparation-related parameter has changed.
The text was updated successfully, but these errors were encountered: