Skip to content

refactor: Move memory_usage into IngressHistoryStats - #11211

Open
alin-at-dfinity wants to merge 1 commit into
masterfrom
alin/memory_usage-into-IngressHistoryStats
Open

refactor: Move memory_usage into IngressHistoryStats#11211
alin-at-dfinity wants to merge 1 commit into
masterfrom
alin/memory_usage-into-IngressHistoryStats

Conversation

@alin-at-dfinity

Copy link
Copy Markdown
Contributor

IngressHistoryState::memory_usage and IngressHistoryState::state_counts are always updated and validated in tandem. Move memory_usage into IngressHistoryStats and rename state_counts to stats, now that it's more generic than just counts by state.

`IngressHistoryState::memory_usage` and `IngressHistoryState::state_counts` are always updated and validated in tandem. Move `memory_usage` into `IngressHistoryStats` and rename `state_counts` to `stats`, now that it's more generic than just counts by state.
@alin-at-dfinity
alin-at-dfinity requested review from mraszyk and a lite review from Copilot August 19, 2026 08:37
@alin-at-dfinity
alin-at-dfinity requested a review from a team as a code owner August 19, 2026 08:37
@zeropath-ai

zeropath-ai Bot commented Aug 19, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 4eb5854.

Security Overview
Detected Code Changes
Change Type Relevant files
Refactor ► rs/replicated_state/src/lib.rs
IngressHistoryState exports updated to match new fields
► rs/replicated_state/src/metadata_state.rs
Replace memory_usage and state_counts with stats field; rename and restructure related methods and debug assertions
► rs/replicated_state/src/metadata_state/proto.rs
Update From/TryFrom implementations to use stats instead of memory_usage/state_counts
Enhancement ► rs/replicated_state/src/metrics.rs
Iterate over state.metadata.ingress_history.state_counts() directly (type adjustments)
Bug Fix ► rs/replicated_state/src/metadata_state.rs
Align debug_asserts and computations to use compute_stats and new stats field instead of old memory_usage/state_counts
► rs/replicated_state/src/metadata_state/tests.rs
Update test to reference ingress_history.stats.memory_usage instead of ingress_history.memory_usage

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors ingress history transient bookkeeping by consolidating memory usage tracking into IngressHistoryStats and renaming the internal “counts” concept into a more general stats aggregation, keeping related invariants updated together.

Changes:

  • Move ingress history memory_usage tracking into IngressHistoryStats and store it under IngressHistoryState.stats.
  • Refactor “counts-by-state” handling to flow through stats, including a single compute_stats() recomputation used in debug assertions and decoding.
  • Update metrics emission and tests to use the refactored state_counts() iteration and new stats layout.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
rs/replicated_state/src/metrics.rs Adjust metrics loop to iterate directly over ingress_history.state_counts() (now returns an iterator).
rs/replicated_state/src/metadata_state/tests.rs Update tests to reference the refactored stats.memory_usage.
rs/replicated_state/src/metadata_state/proto.rs Update debug validation and decoding to recompute/assign the unified stats.
rs/replicated_state/src/metadata_state.rs Consolidate transient ingress history bookkeeping into stats, add compute_stats(), and update internal update paths.
rs/replicated_state/src/lib.rs Stop re-exporting IngressHistoryStats after it becomes an internal implementation detail.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants