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: make asset canister reserve space when saving to stable storage #4036

Merged
merged 13 commits into from
Dec 11, 2024

Conversation

ericswanson-dfinity
Copy link
Member

@ericswanson-dfinity ericswanson-dfinity commented Dec 11, 2024

Description

Make the asset canister estimate the size of the data to be serialized to stable memory, and reserve that much space for the ValueSerializer's buffer.

The portal fails to upgrade from dfx 0.22.0 with an opaque Canister trapped: unreachable error.

This turns out to be the chain of events:

  • asset canister calls ic_cdk::storage::stable_save, which in turn calls candid::write_args, which constructs an IDLBuilder and serializes the data onto it.
  • The IDLBuilder contains a ValueSerializer, which contains an empty Vec<u8> with 0 capacity.
  • Serialization of the portal's data, which is just over 1GB, eventually tries to grow this Vec when it is already near 1 GB.
  • This generates an AllocError.

Part of https://dfinity.atlassian.net/browse/SDK-1827

How Has This Been Tested?

Create a new project and add the portal's data to its frontend canister. I ended up reading the data from the portal canister.

In one shell, run the following:

while true ; do dfx +0.24.3 start --clean -v ; done

In another shell, with the dfx under test on the PATH, run the following:

dfx killall && dfx ping --wait-healthy && dfx deploy && dfx build && dfx canister install scraped-portal-frontend --mode upgrade --upgrade-unchanged -vv

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@ericswanson-dfinity ericswanson-dfinity marked this pull request as ready for review December 11, 2024 18:12
@ericswanson-dfinity ericswanson-dfinity requested a review from a team as a code owner December 11, 2024 18:12
lwshang
lwshang previously approved these changes Dec 11, 2024
@ericswanson-dfinity ericswanson-dfinity merged commit abf4540 into master Dec 11, 2024
297 checks passed
@ericswanson-dfinity ericswanson-dfinity deleted the ens/sdk-1827-upgrade-asset-canister branch December 11, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants