feat: Add replicated state metrics for ingress history, queues, call contexts and refunds - #11010
Draft
mraszyk wants to merge 6 commits into
Draft
feat: Add replicated state metrics for ingress history, queues, call contexts and refunds#11010mraszyk wants to merge 6 commits into
mraszyk wants to merge 6 commits into
Conversation
Adds Prometheus metrics for a number of replicated state quantities that were not observable so far: * `replicated_state_ingress_history_length_by_state`, the number of ingress history entries by ingress state. In order to avoid scanning the whole ingress history (up to `INGRESS_HISTORY_MAX_MESSAGES` entries) on every round, `IngressHistoryState` maintains the counts incrementally, in a new transient field, just like `memory_usage` (recomputed on deserialization and on subnet splitting; validated against a full recount in debug builds). * `execution_output_queue_messages`, the number of messages in canister output queues; plus `execution_subnet_input_queue_messages` and `execution_subnet_output_queue_messages` for the subnet (i.e. management canister) queues, which the existing `execution_input_queue_messages` does not cover. * `replicated_state_subnet_call_contexts`, the number of in-progress subnet calls by type (canister HTTP request, stop canister, etc.). * `replicated_state_pending_refunds`, the number of pending anonymous refunds in the subnet-level refund pool. * `replicated_state_unresponded_unbounded_wait_call_contexts`, the number of unresponded call contexts with a pending unbounded-wait response, by the sender canister's subnet; in order to be able to tell whether a dropped unbounded-wait response could result in a critical error later on. Computing this is `O(1)` for canisters without any such call context. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mraszyk
changed the title
Conflict in `rs/replicated_state/src/metadata_state/tests.rs`: both sides added imports from `subnet_call_context_manager` (union of both). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves observability of replicated state health by adding Prometheus metrics for several replicated-state quantities that are relevant for validating “state-only” subnet merges (copying canister states/snapshots without other subnet state).
Changes:
- Add replicated-state metrics for ingress history size by ingress state, canister/subnet queue message counts (including output queues), subnet call context counts by type, pending subnet refunds, and unresponded unbounded-wait call contexts grouped by sender subnet.
- Extend
IngressHistoryStatewith a transient, incrementally-maintainedIngressStateCounts(recomputed on deserialization/splitting; debug-validated). - Add APIs and tests to support/validate the new metrics (
SubnetCallContextManager::context_counts,CallContextManager::for_each_unresponded_unbounded_wait_originator, and new metric-focused tests).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rs/replicated_state/src/metrics.rs | Registers and observes the new replicated-state metrics (ingress-by-state, queues, subnet call contexts, refunds, unbounded-wait call contexts). |
| rs/replicated_state/src/metadata_state/tests.rs | Adds unit tests for ingress state counts and subnet call context manager context counting. |
| rs/replicated_state/src/metadata_state/subnet_call_context_manager.rs | Adds context_counts() iterator to expose counts by subnet-call type. |
| rs/replicated_state/src/metadata_state/proto.rs | Recomputes and debug-validates ingress state counts on (de)serialization. |
| rs/replicated_state/src/metadata_state.rs | Introduces IngressStateCounts transient field and keeps it updated across ingress history mutations. |
| rs/replicated_state/src/lib.rs | Re-exports IngressStateCounts. |
| rs/replicated_state/src/canister_state/system_state/call_context_manager/tests.rs | Adds unit test for iterating originators of unresponded unbounded-wait call contexts. |
| rs/replicated_state/src/canister_state/system_state/call_context_manager.rs | Adds for_each_unresponded_unbounded_wait_originator() to support the new metric efficiently. |
| rs/execution_environment/src/scheduler/tests/metrics.rs | Adds integration tests verifying the new replicated-state metrics are exported with expected values/labels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`replicated_state_unresponded_unbounded_wait_call_contexts` only counts call contexts that have not been responded to yet. Cover that distinction: * At the metric level, by responding to one of the counted call contexts and observing that the count drops. The call context is made to survive being responded to (by first making a downstream call on its behalf), so that the count drops because of the response, rather than because the call context was dropped altogether. Verified by mutation: without the `responded` check in `for_each_unresponded_unbounded_wait_originator()`, the assertion fails. * At the unit level, by additionally asserting that the call context dropped from the reported originators is indeed still around. Also clarifies the `IngressStateCounts` doc comment as to why `IngressStatus::Unknown` is counted even though it never describes an entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`IngressStatus::Unknown` stands for the absence of an ingress history entry, so it must never be recorded as the status of a message. The `IngressStatus::is_valid_state_transition()` checks done by the callers of `IngressHistoryState::insert()` only catch this if an entry already exists, as any transition away from `Unknown` (i.e. from "no entry") is allowed. Assert it at the bottom of the stack instead, where the entry is actually recorded. The `unknown` count of `IngressStateCounts` is retained as a release build backstop, so the counts still add up to the number of entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restructures the tests for the newly added replicated state metrics along a single strategy: starting from a fresh state, update the state so that the metric (or one label value thereof, with all the others at zero) becomes 1; then update the state again, so that it drops back to 0. Isolating one label value at a time also ensures that each of them is wired up to the right thing. The `subnet_call_contexts` metric is covered in `ic-replicated-state`, as `ic-execution-environment` does not depend on `ic-btc-replica-types` (needed in order to construct the Bitcoin contexts). Retains the tests that cover more than the above: * `ingress_history_state_counts`, covering the transitions between ingress states (including overwriting an entry, forgetting a terminal status and pruning); * `test_for_each_unresponded_unbounded_wait_originator`, covering the call contexts that are not counted (best-effort and ingress origins); * `ingress_history_insert_unknown_status_panics`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`IngressStatus::Unknown` stands for the absence of an ingress history entry, so recording one is `debug_assert`ed against in `IngressHistoryState::insert()`. Three test fixtures did just that: * `test_traverse_ingress_history` recorded one to cover the canonical state encoding of `Unknown`, which is unreachable for the same reason. Drop the entry along with the corresponding expected traversal. * `valid_transitions()` listed `Unknown` both as an origin and as a target status. Represent the origin as an `Option<IngressStatus>`, where `None` stands for "no ingress history entry" (which is what `Unknown` means), and drop it from the targets. `test_invalid_transitions` thus still covers `Unknown` as an invalid target from every origin, including from an empty ingress history, which is exactly what the `debug_assert` catches. * the `test_backward_compatibility` state fixture recorded five of them. Record the five ingress states that actually can be recorded instead, which also covers the `reply` and `Completed(Reject)` canonical encodings that the fixture was missing. The partial state hashes change accordingly: the fixture changed, the hashing did not. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Adds Prometheus metrics for a number of replicated state quantities that were not observable so far, but are necessary to confirm that a subnet can be merged with another one by only copying over canister states and canister snapshots:
replicated_state_ingress_history_length_by_state, the number of ingress history entries by ingress state. In order to avoid scanning the whole ingress history (up toINGRESS_HISTORY_MAX_MESSAGESentries) on every round,IngressHistoryStatemaintains the counts incrementally, in a new transient field, just likememory_usage(recomputed on deserialization and on subnet splitting; validated against a full recount in debug builds).execution_output_queue_messages, the number of messages in canister output queues; plusexecution_subnet_input_queue_messagesandexecution_subnet_output_queue_messagesfor the subnet (i.e. management canister) queues, which the existingexecution_input_queue_messagesdoes not cover.replicated_state_subnet_call_contexts, the number of in-progress subnet calls by type (canister HTTP request, stop canister, etc.).replicated_state_pending_refunds, the number of pending anonymous refunds in the subnet-level refund pool.replicated_state_unresponded_unbounded_wait_call_contexts, the number of unresponded call contexts with a pending unbounded-wait response, by the sender canister's subnet; in order to be able to tell whether a dropped unbounded-wait response could result in a critical error later on. Computing this isO(1)for canisters without any such call context.