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

fix: [EXC-1831] Low Wasm memory hook will run once the canister's unfrozen if it was scheduled before freezing #3455

Conversation

dragoljub-duric
Copy link
Contributor

@dragoljub-duric dragoljub-duric commented Jan 15, 2025

Problem:
@mraszyk noticed here that property:
”If the canister gets frozen immediately after the function is scheduled for execution, the function will run once the canister's unfrozen if the canister's wasm memory remains above the threshold t.”
is missing.

Solution:
When execution of the hook is stopped because of the freezing canister, add the hook again to the task queue.

@github-actions github-actions bot added the fix label Jan 15, 2025
@dragoljub-duric
Copy link
Contributor Author

An alternative approach was, that instead of always updating the hook status to Executed whenever task_queue::pop_front() is called and the hook status is Ready, we keep the hook status Ready after task_queue::pop_front() is called and only once its execution is finished we update the hook status to Executed. But in my opinion conceptually that will not be so nice, because TaskQueue::pop_front() will not be popping anything.

@dragoljub-duric dragoljub-duric marked this pull request as ready for review January 15, 2025 11:30
@dragoljub-duric dragoljub-duric requested a review from a team as a code owner January 15, 2025 11:30
@dragoljub-duric dragoljub-duric changed the title fix: EXC-1831 If canister gets frozen immediately after the hook is scheduled for execution, the hook will run once the canister's unfrozen fix: [EXC-1831] Low Wasm memory hook will run once the canister's unfrozen if it was scheduled before freezing Jan 15, 2025
Copy link
Contributor

@berestovskyy berestovskyy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, modulo one more error path.

@dragoljub-duric dragoljub-duric added this pull request to the merge queue Jan 16, 2025
Merged via the queue into master with commit 57047d6 Jan 16, 2025
31 checks passed
@dragoljub-duric dragoljub-duric deleted the EXC-1831-if-canister-gets-frozen-immediately-after-the-hook-is-scheduled-for-execution-the-hook-will-run-once-the-canisters-unfrozen branch January 16, 2025 10:00
github-merge-queue bot pushed a commit that referenced this pull request Feb 3, 2025
… fixed (#3631)

Problem:
As previously observed by @berestovskyy
#3455 (comment) it may
happen that execution of low_wasm_memory hook is stopped when
`wasm_memory_limit` < `used_wasm_memory`.
Solution:
If that happens, run the hook after the error is fixed if the hook
condition remains satisfied.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants