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

refactor: [EXC-1752] Remove the parts of old CanisterStateBits after migration #4335

Open
wants to merge 43 commits into
base: master
Choose a base branch
from

Conversation

dragoljub-duric
Copy link
Contributor

@dragoljub-duric dragoljub-duric commented Mar 12, 2025

This is follow-up on #2254. The plan is to merge this PR after the cutoff for this week's release.

@dragoljub-duric dragoljub-duric changed the base branch from master to EXC-1752-introduce-canister-state-bits-version-of-task-queue March 12, 2025 13:15
github-merge-queue bot pushed a commit that referenced this pull request Mar 12, 2025
This PR adds `TaskQueue` to `CanisterStateBits`. To keep the change
backward compatible, we are adding new optional field tasks of type
`TaskQueue` to `pb::CanisterStateBits`.

In this PR we are adding support to deserialize both protobuf versions
(old and new), while we always serialize with the new one. In the
[follow-up](#4335) (planed for the
next release) we will remove(reserve) `pb::CanisterStateBits` fields
`on_low_wasm_memory_hook_status: pb::OnLowWasmMemoryHookStatus` and
`task_queue: pb::VecDequeue` and we will remove deserialization with the
old protobuf version.

---------

Co-authored-by: Dimitris Sarlis <dimitrios.sarlis@dfinity.org>
Base automatically changed from EXC-1752-introduce-canister-state-bits-version-of-task-queue to master March 12, 2025 14:53
@dsarlis
Copy link
Member

dsarlis commented Mar 12, 2025

The plan is to merge this PR after the cutoff for this week's release.

I would wait until next week when it would be more clear that we don't need to roll back the previous change. There's no reason to risk anything earlier.

@schneiderstefan
Copy link
Member

I recommend you manually run the nightly tests at some point. The upgrade_downgrade system tests work by upgrading between master and the version currently running on the NNS subnet. There are 2 possible outcomes, and I don't know which one you'll get:

  1. The test passes. This means the workload that we use for the test doesn't involve any task queues (or they are dropped silently). Possibly a testing gap that can be fixed.
  2. The test fails. In this case you need to wait with the merge until the first PR hits the NNS subnet (1.5 weeks from now). Otherwise nightly will start failing.

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.

None yet

3 participants