test: drain a subnet that is "cooling down" - #11371
Draft
mraszyk wants to merge 21 commits into
Draft
Conversation
If the `subnet_merged` marker is set, `process_batch()` now resets it and records all not yet responded ingress-induced canister call contexts as `Processing` in the ingress history, so that the corresponding ingress messages can be tracked to completion. Subnet call contexts are ignored, as subnet merging ensures that the subnets being merged have no in-progress subnet call contexts. A message that already has an ingress history entry is left alone. Its status is expected to be `Processing`; anything else raises the new `mr_unexpected_ingress_status_after_merge` critical error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a `merge_subnets` endpoint to the registry canister. It takes a source and a destination subnet ID and merges the canister ID ranges of the source subnet into the canister ID range set of the destination subnet, so that the canisters hosted by the source subnet are routed to the destination subnet afterwards. The routing table is updated via `modify_routing_table`, which assigns the source subnet's ranges to the destination subnet and coalesces the resulting adjacent ranges. Only the routing table is touched: neither subnet record is modified and the source subnet is not deleted, so decommissioning a subnet is `merge_subnets` followed by `delete_subnet`. The endpoint is restricted to governance and rejects payloads where the two subnet IDs are equal, either subnet is unknown, the source subnet hosts no canister ID range, or an ongoing canister migration overlaps the source subnet's ranges (which would leave those migrations with a host that is not on their recorded trace). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A system test covering the `cooling_down` subnet record flag end to end: while two universal canisters keep calling each other across subnets in a loop, labeling one of their subnets as "cooling down" must bring that subnet to a standstill. The IC consists of an NNS subnet (with the NNS canisters installed) and two Application subnets S and T, holding one universal canister each. Both canisters are made to call the one on the other subnet in a loop, where the reply (or reject) callback of every call fires a new call. A payload cannot contain itself, so each canister holds the loop body in its global data and the continuation calls the canister itself, passing the global data as the payload to execute; the loop body replies as soon as it has fired the cross-subnet call, which keeps the number of open call contexts bounded. An `UpdateConfigOfSubnet` proposal then labels S as "cooling down" and the test waits until S rejects ingress messages and, subsequently, until it is "merge ready" according to the `Merge readiness` panel of the `Subnet merging` dashboard (`bases/apps/ic-dashboards/core/subnet-merging.json` in dfinity-ops/k8s) for `V` = the registry version created by the proposal and `R` = 0 cycles: every subnet has reached V, no stream in either direction holds a message (loopback included), the ingress history holds nothing but `processing` entries, S's subnet input and output queues are empty, its subnet call context manager holds no call context, and the pending anonymous refunds are worth at most R. As in the dashboard, each term is evaluated on the median across the replicas reporting it, and missing data reads as zero. Finally, the test checks that both loops are stalled, so that readiness is the consequence of S cooling down rather than of the loops having stopped. Only the HEAD NNS variant runs: the mainnet NNS canisters do not know the `cooling_down` field of `UpdateSubnetPayload`, so Candid would silently drop it and the proposal would be a no-op. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Extend the `cooling_down` system test with the kinds of work a subnet may be
busy with when it is labeled "cooling down", and rename the subnets after the
roles they would play in a merge: `M` is the subnet being merged, `R` the root
subnet it would be merged into, `T` the subnet `M` exchanges messages with in a
loop, and `S` a third Application subnet, newly added.
Besides the two universal canisters calling each other across the `M`/`T`
boundary in a loop, `M` now also holds:
- `U1`, which makes five `install_code` calls to the management canister, one
per empty canister `U2a` .. `U2e` it controls, installing the universal
canister module with an `arg` that makes `canister_init` burn 295B
instructions. Only one long-running `install_code` makes progress per round,
so these five calls alone keep `M` busy for minutes and are what merge
readiness now waits for, rather than the pruning of the ingress history.
- `U3`, looping until its global data is set to a value this test never sets.
Every iteration is a self-targeted `canister_status` call, so the loop stalls
as soon as `M` stops routing messages out of its canisters' output queues.
- `U5`, running that same loop in a call made from `U4` on `S`, i.e. in a call
from another subnet that it can never respond to.
- `U6`, waiting for the response of a call to `U7` on `S` that never arrives.
The test then asserts that `U2a` .. `U2e` have been installed once `M` is merge
ready, i.e. that cooling down retains messages in canister output queues without
losing the management calls `M` had already accepted. The endless loops leave
nothing behind but `processing` ingress history entries and open canister call
contexts, neither of which is part of the readiness condition.
Two ordering constraints the test has to respect, both of which would otherwise
make it hang or install nothing:
- Every canister has to be installed before `U1`'s calls start, as no
`install_code` is executed at all while another one is long-running on the
same subnet.
- `U1`'s requests have to leave its output queue before the proposal lands: a
request still sitting there would never be routed once `M` cools down, not
even into the loopback stream.
`canister_init` burns 295B rather than the full 300B an `install_code` message
may consume, because that budget also covers compiling the module: 6_000
instructions per byte of the decompressed (~350 KB) universal canister module,
i.e. ~2.2B instructions, plus a 20M base cost. The reduced compilation cost only
applies while the module is in `expected_compiled_wasms`, which is cleared at
every checkpoint, so an `install_code` aborted at a checkpoint and restarted
afterwards pays the full amount.
Supporting changes: `CandidCallBuilder<InstallCodeArgs>` grows a `with_arg`, as
there was no way to set the argument passed to `canister_init`, and
`UniversalCanister` grows a `submit_update` that submits an ingress message
without waiting for it to complete, which most of the calls above need.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…bnet-cooling-down-test'
…merge
Add the `MergeSubnets` NNS function, mapped to the `merge_subnets` method of the
registry canister under the `SubnetManagement` topic, and extend that method so
that a single such proposal performs the whole registry-side part of a subnet
merge, all in one registry version, so that the destination subnet never
observes a state where only some of it took effect:
1. the canister ID ranges of the source subnet are merged into the canister ID
range set of the destination subnet, so that all canisters that used to be
hosted by the source subnet are routed to the destination subnet;
2. a recovery catch-up package is created for the destination subnet, at the
height, time and state hash of the merged state, running a fresh DKG for
the destination subnet's membership; and
3. the destination subnet is brought back online.
The payload therefore grows the `height`, `time_ns` and `state_hash` of the
merged state, plus an `initial_dkg_subnet_id` naming the subnet that handles the
`setup_initial_dkg` call: it must not be the destination subnet, which is
offline while the merge is in progress.
The caller is expected to have taken both subnets offline and to have extended
the state of the destination subnet with the state of the canisters of the source
subnet beforehand; `state_hash` is the hash of the manifest of the result. The
source subnet record is not modified and the source subnet is not deleted: it
merely does not host any canister ID range anymore.
The recovery catch-up package needs fresh DKG transcripts because its
`initial_ni_dkg_transcript_{low,high}_threshold` become the current transcripts
of the DKG summary that bootstraps consensus at the recovery height. The registry
only holds the destination subnet's genesis (or last recovery) catch-up package,
whose transcripts its nodes may no longer have the secret key shares for, so
reusing those would risk restarting a subnet that cannot sign.
Merging into a subnet holding chain keys is rejected: recovering such a subnet
requires resharing its keys onto the recovery catch-up package, which this
method does not do, and silently leaving the destination subnet unable to sign
would be worse than refusing.
As the method now makes an inter-canister call half way through, it follows
`do_recover_subnet` in checking that none of the records it is about to
overwrite -- and none of the canister ID ranges it validated -- changed while
that call was in flight. Unlike `do_recover_subnet`, it reports the reject code
and message if that call fails, rather than an opaque `unwrap` panic.
`StateMachine` only answered `setup_initial_dkg` requests from
`do_execute_round`, which `tick()` (and hence `await_ingress`) does not go
through, so any canister awaiting such a call hung there forever. Factor the
fake responses out into `setup_initial_dkg_responses` and produce them from
`tick_with_config` as well, next to the threshold signing requests it already
answers. Without this the integration test of the success path cannot run, which
is why it is part of this commit.
The two success unit tests of `merge_subnets`, which can no longer drive the
method to completion, now exercise the routing table part directly; the success
path as a whole, including the recovery catch-up package and the unhalting, is
covered by the integration test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add a `checkpoint_time` subcommand printing the `batch_time_nanos` of the system metadata of a checkpoint, i.e. the IC time the subnet had reached when it wrote that checkpoint. Merging a subnet into another one has to pick the block time the destination subnet resumes from, which must be larger than the times of the checkpoints at which both subnets were taken offline. Nothing printed a checkpoint's time so far, so there was no way to check that from outside the replica. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o batches A subnet halting because of the `halt_at_cup_height` flag of its subnet record logged nothing observable: the only trace it left was a `debug!`, which the nodes' log level drops, so nothing outside the replica could tell that such a subnet had come to a stop. The `is_halted` flag, in contrast, is reported at info level by `ConsensusImpl::on_state_change`. Raise that message to info level, so that both ways of halting a subnet can be observed the same way. It is already emitted at most once every five seconds, so this does not make the logs noticeably busier, and it names the height of the first batch that is not delivered, i.e. one past the state the subnet stopped in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Extend the `cooling_down` system test past the point where the subnet is "merge
ready": it now goes on to merge that subnet away and to delete it, which is what
the cooling down and the merge readiness condition exist for. The destination is
a third Application subnet `R`; the NNS subnet stays available throughout, as it
is where the proposals of the test are executed, including the one recovering
`R` at the merged state.
The merge itself, once both subnets are "merge ready":
- Both subnets are told to halt at their next CUP and are waited for. The wait
reads each node's journal, where a halted replica reports that it delivers
no batches, rather than watching for the height of the latest checkpoint to
stop advancing: checkpoints are written once per DKG interval, i.e. minutes
apart, so that height looks stable long before the subnet halts. Its
checkpoint is then hundreds of rounds behind the state the merge readiness
was established on, and holds, for instance, an `install_code` aborted at
that checkpoint and only completed afterwards -- whose call context and
reserved response slot the merge then drops, so that the destination subnet
hits `[EXC-BUG] Could not find any install code call ...` and panics on
resuming it.
- The state of the merge is assembled from the two checkpoints the subnets
halted at, as a new checkpoint of `R` at the next multiple of its DKG
interval, so that `R`'s own checkpoint stays untouched: the canisters and
canister snapshots of `M` are added to those of `R`, and the result is marked
as the product of a subnet merge. Taking the system metadata and subnet
queues of `R` wholesale is sound because those of `M` are empty, which is
what the merge readiness established and what `M` cooling down preserves
until it halts.
- The block time of the merged state is picked past both checkpoint times,
read with `state-tool checkpoint_time`, and its manifest hash with
`state-tool manifest`.
- The merged state is added to `R`'s checkpoints while its replica is stopped,
and the replica is only started after the `MergeSubnets` proposal created the
recovery CUP; the test then waits for `R` to report exactly that CUP, so that
a subnet resuming from the wrong state fails here rather than much later.
- The three endless loops are ended and every ingress message that was in
progress across the merge is checked to have completed. This is what the
`subnet_merged` marker is for: the ingress history of `M` is deliberately not
merged in, the marker makes the replica re-register those messages instead.
- Finally, once every subnet observed the merge, `M` -- which hosts no canister
ID range anymore -- is deleted, and is checked to be gone from the registry.
`UniversalCanister::submit_update` now returns the ID of the message it
submitted, which is what lets the test track the ingress messages that outlive
the merge.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # rs/registry/canister/unreleased_changelog.md
Strengthen the subnet merge system test along the axes it was blind to. Every subnet now has four nodes rather than one, so that the merge has to get the merged state to the other nodes of the destination subnet the way a recovery does, i.e. by state sync from the one node it is uploaded to, and so that the medians the merge readiness condition is made of are medians of more than one value. The destination subnet is no longer empty: it hosts `UR`, which the merge has to leave alone even though the canisters of the merged subnet are added right next to it, and which has to be able to call one of those canisters afterwards, both of them being on the same subnet from then on. `U8` carries the state that has to survive the merge: a blob in its stable memory, a canister snapshot, and a cycles balance, all three of which are checked once the destination subnet serves it. The balance is compared against a share of itself rather than an amount, as the two readings are however many minutes apart the waits in between take, and what this catches is a balance that was not carried over at all rather than the resource charges of an idle canister. The merge readiness condition is now checked to *not* hold right after the subnet starts cooling down. A condition that held from the start would be satisfied by a subnet that never had anything to drain, so waiting for it would prove nothing; six of its eight terms are in fact unsatisfied at that point. Finally, a best effort call from `U9` on `T` to `U10` on `M`, which `U10` never answers, puts a best effort message in flight across the merge: its deadline passes while `M` is cooling down, and the cycles it carries are held by the open call context of its callee, which the merge has to carry over like any other canister state. The test had only guaranteed response calls until now. The one term of the readiness condition that stays untested is the pending anonymous refunds, which requires a cycle bearing message to be dropped from one of the subnet's queues while owed to a canister of another subnet; the cycles of the best effort call above are not it, as that call is picked up rather than dropped. See the comment on `MAX_REFUND_VALUE_CYCLES`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…merge
Add the `MergeSubnets` NNS function, mapped to the `merge_subnets` method of the
registry canister under the `SubnetManagement` topic, and extend that method so
that a single such proposal performs the whole registry-side part of a subnet
merge, all in one registry version, so that the destination subnet never
observes a state where only some of it took effect:
1. the canister ID ranges of the source subnet are merged into the canister ID
range set of the destination subnet, so that all canisters that used to be
hosted by the source subnet are routed to the destination subnet;
2. a recovery catch-up package is created for the destination subnet, at the
height, time and state hash of the merged state, running a fresh DKG for
the destination subnet's membership; and
3. the destination subnet is brought back online.
The payload therefore grows the `height`, `time_ns` and `state_hash` of the
merged state, plus an `initial_dkg_subnet_id` naming the subnet that handles the
`setup_initial_dkg` call: it must not be the destination subnet, which is
offline while the merge is in progress.
The caller is expected to have taken both subnets offline and to have extended
the state of the destination subnet with the state of the canisters of the source
subnet beforehand; `state_hash` is the hash of the manifest of the result. The
source subnet record is not modified and the source subnet is not deleted: it
merely does not host any canister ID range anymore.
The recovery catch-up package needs fresh DKG transcripts because its
`initial_ni_dkg_transcript_{low,high}_threshold` become the current transcripts
of the DKG summary that bootstraps consensus at the recovery height. The registry
only holds the destination subnet's genesis (or last recovery) catch-up package,
whose transcripts its nodes may no longer have the secret key shares for, so
reusing those would risk restarting a subnet that cannot sign.
Merging into a subnet holding chain keys is rejected: recovering such a subnet
requires resharing its keys onto the recovery catch-up package, which this
method does not do, and silently leaving the destination subnet unable to sign
would be worse than refusing.
As the method now makes an inter-canister call half way through, it follows
`do_recover_subnet` in checking that none of the records it is about to
overwrite -- and none of the canister ID ranges it validated -- changed while
that call was in flight. Unlike `do_recover_subnet`, it reports the reject code
and message if that call fails, rather than an opaque `unwrap` panic.
`StateMachine` only answered `setup_initial_dkg` requests from
`do_execute_round`, which `tick()` (and hence `await_ingress`) does not go
through, so any canister awaiting such a call hung there forever. Factor the
fake responses out into `setup_initial_dkg_responses` and produce them from
`tick_with_config` as well, next to the threshold signing requests it already
answers. Without this the integration test of the success path cannot run, which
is why it is part of this commit.
The two success unit tests of `merge_subnets`, which can no longer drive the
method to completion, now exercise the routing table part directly; the success
path as a whole, including the recovery catch-up package and the unhalting, is
covered by the integration test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # rs/registry/canister/unreleased_changelog.md
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Revalidate, after `setup_initial_dkg` returns, that no canister migration overlapping the canister ID ranges of the source subnet was prepared while the call was in flight: `prepare_canister_migration` does not touch the routing table, so the existing post-call check could not catch it. Also fix the low/high threshold transcript records being swapped in the `SetupInitialDKGResponse` that `StateMachine` synthesizes for pending `setup_initial_dkg` requests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Chain key initializations in a CUP take precedence over the chain key configuration of the subnet record, so carrying over the ones of the CUP being replaced would make the destination subnet bootstrap obsolete key material -- contradicting the endpoint's refusal to merge into a subnet holding chain keys. Clear both initialization fields, as recovering a subnet without an initial chain key configuration does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Revert `merge_subnets` to what it originally did: merge the canister ID ranges of the source subnet into the canister ID range set of the destination subnet, and nothing else. The recovery catch-up package, the fresh DKG for the destination subnet's membership and the unhalting are dropped again, and with them the `height`, `time_ns`, `state_hash` and `initial_dkg_subnet_id` payload fields, the inter-canister call and the guards around it. Recovering the destination subnet at the merged state stays with `recover_subnet`. The `MergeSubnets` NNS function is kept -- without it the endpoint would be unreachable -- with its description narrowed accordingly. The `StateMachine` change that answered `setup_initial_dkg` requests from `tick_with_config` goes away with the inter-canister call that needed it. Its fix of the swapped `SetupInitialDKGResponse` fields stays, as that is a bug of its own: the `HighThreshold`-tagged transcript was encoded as the low-threshold record and vice versa. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…wn-test' `merge_subnets` is restricted to the routing table change again, so the registry side of the merge is taken from the merged-in branch wholesale: the endpoint, its payload, the Candid interfaces, the changelogs and the `StateMachine` handling of `setup_initial_dkg` requests. The extraction of `setup_initial_dkg_responses` goes away with the inter-canister call that needed it; the fix of the swapped `SetupInitialDKGResponse` fields stays. The system test now performs the merge with two proposals instead of one: `MergeSubnets` reroutes the canister ID ranges of `M` to `R`, and `RecoverSubnet` creates the recovery CUP for `R` at the merged state. Recovering a subnet that was instructed to halt at its next CUP replaces that instruction with a plain halt, so `R` is unhalted with an `UpdateConfigOfSubnet` proposal once it came up on the recovery CUP -- before the ingress messages that outlived the merge are checked to complete, as a halted subnet delivers no batches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…time `await_status_is_healthy` and the SSH helpers without the `_async` suffix run their own future through `futures::executor::block_on`, which polls it on the calling thread instead of letting the runtime park on its I/O. A `reqwest` request that finds no live connection in its pool then busy-polls its connect future forever: the test spun at 100% CPU in `status_is_healthy_async`, having logged the unhalting of `R` and nothing after it. `R` reports `WaitingForRootDelegation` for a few minutes after the recovery, which is what gave the pooled connection time to go away, so the health check of step 16 hit this reliably. Use the `_async` variants -- which is what this file already does for the checkpoint listing of `latest_checkpoint_height` -- for every driver call in the async body of the test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A system test covering the
cooling_downsubnet record flag end to end: while two universal canisters keep calling each other across subnets in a loop, labeling one of their subnets as "cooling down" must bring that subnet to a standstill.The IC consists of an NNS subnet (with the NNS canisters installed) and two Application subnets S and T, holding one universal canister each. Both canisters are made to call the one on the other subnet in a loop, where the reply (or reject) callback of every call fires a new call. A payload cannot contain itself, so each canister holds the loop body in its global data and the continuation calls the canister itself, passing the global data as the payload to execute; the loop body replies as soon as it has fired the cross-subnet call, which keeps the number of open call contexts bounded.
An
UpdateConfigOfSubnetproposal then labels S as "cooling down" and the test waits until S rejects ingress messages and, subsequently, until it is "merge ready" according to theMerge readinesspanel of theSubnet mergingdashboard (bases/apps/ic-dashboards/core/subnet-merging.jsonin dfinity-ops/k8s) forV= the registry version created by the proposal andR= 0 cycles: every subnet has reached V, no stream in either direction holds a message (loopback included), the ingress history holds nothing butprocessingentries, S's subnet input and output queues are empty, its subnet call context manager holds no call context, and the pending anonymous refunds are worth at most R. As in the dashboard, each term is evaluated on the median across the replicas reporting it, and missing data reads as zero. Finally, the test checks that both loops are stalled, so that readiness is the consequence of S cooling down rather than of the loops having stopped.Only the HEAD NNS variant runs: the mainnet NNS canisters do not know the
cooling_downfield ofUpdateSubnetPayload, so Candid would silently drop it and the proposal would be a no-op.