fix: run the outcall cycles migration regardless of subnet activity - #11219
Open
mraszyk wants to merge 1 commit into
Open
fix: run the outcall cycles migration regardless of subnet activity#11219mraszyk wants to merge 1 commit into
mraszyk wants to merge 1 commit into
Conversation
The migration added in #10844, which brings the HTTPOutcalls / ECDSAOutcalls entries of SubnetMetrics::consumed_cycles_by_use_case up to the legacy scalar fields, was invoked from observe_consumed_cycles_with_use_case. That only fires when the subnet observes a subnet-level use case, and those are only observed on HTTP outcalls, threshold signature outcalls, canister deletion and cycles lost to dropped messages. A subnet that does none of these never runs the migration and its use-case entries stay stale indefinitely. This is not hypothetical: on mainnet subnet 2fq7c-slacv-26cgz-vzbx2-2jrcs-5edph-i5s2j-tck77-c3rlz-iobzx-mqe, consumed_cycles_ecdsa_outcalls is 244_900_000_000_000 while the ECDSAOutcalls use-case entry is 228_370_000_000_000, a gap of 16_530_000_000_000 cycles. Its consumed_cycles_by_use_case_as_counters map is empty, which (since that map is incremented on the very same code path) shows that no subnet-level use case has been observed there since the counters were introduced in #9922, i.e. well before the migration was deployed. The migration now runs from the scheduler's finish_round, which is documented as running unconditionally after each round and is also reached by the heap-delta early return and by checkpoint_round_with_no_execution. Running it once per round instead of per observation is equivalent, because the call sites bump the scalar field and the matching use-case entry by the same amount: max(entry, scalar) + delta == max(entry + delta, scalar + delta). It is also idempotent, so the extra invocations are harmless. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to ab33d4a. Security Overview
Detected Code Changes
|
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.
The migration added in #10844, which brings the HTTPOutcalls / ECDSAOutcalls entries of SubnetMetrics::consumed_cycles_by_use_case up to the legacy scalar fields, was invoked from observe_consumed_cycles_with_use_case. That only fires when the subnet observes a subnet-level use case, and those are only observed on HTTP outcalls, threshold signature outcalls, canister deletion and cycles lost to dropped messages. A subnet that does none of these never runs the migration and its use-case entries stay stale indefinitely.
This is not hypothetical: on mainnet subnet
2fq7c-slacv-26cgz-vzbx2-2jrcs-5edph-i5s2j-tck77-c3rlz-iobzx-mqe, consumed_cycles_ecdsa_outcalls is 244_900_000_000_000 while the ECDSAOutcalls use-case entry is 228_370_000_000_000, a gap of 16_530_000_000_000 cycles. Its consumed_cycles_by_use_case_as_counters map is empty, which (since that map is incremented on the very same code path) shows that no subnet-level use case has been observed there since the counters were introduced in #9922, i.e. well before the migration was deployed.
The migration now runs from the scheduler's finish_round, which is documented as running unconditionally after each round and is also reached by the heap-delta early return and by checkpoint_round_with_no_execution. Running it once per round instead of per observation is equivalent, because the call sites bump the scalar field and the matching use-case entry by the same amount: max(entry, scalar) + delta == max(entry + delta, scalar + delta). It is also idempotent, so the extra invocations are harmless.