refactor: remove legacy canister_log and drop log memory store feature flag#10714
Merged
Conversation
…e flag The one-time migration from the legacy `CanisterLog` store to the `LogMemoryStore` has completed on all subnets, so the migration scaffolding and the legacy store can be removed. - Drop the `log_memory_store_feature` flag (and the `LOG_MEMORY_STORE_FEATURE`/`_ENABLED` consts); the log memory store is now always enabled. Thread-through removed from the scheduler, canister manager, query handler and system state constructors. - Remove the `SystemState.canister_log` field and the `CanisterState` log wrappers now operate solely on `log_memory_store`. - Remove the migration scaffolding: the per-canister `migrated` flag (`is_migrated`/`set_migrated`/`clear_migrated`), the subnet-level `logs_migrated` metadata, the scheduler migration round, the `filter_canister_log_records` helper and the migration-duration metric. Reads, writes and metrics now use `log_memory_store` unconditionally. - Stop serializing `canister_log_records`, `next_canister_log_record_idx` and `log_memory_store_migrated`; the corresponding proto fields (43, 44, 66) are marked reserved. The `CanisterLog` type and the transient `SystemStateModifications` delta log buffer are retained. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collapse the multi-line `reserved` names statement onto a single line so `buf format` (//pre-commit:buf-check) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e accessors Replace the CanisterLog-returning test helper with canister_log_records and canister_log_bytes_used, reading directly from the LogMemoryStore. Add a test-only LogMemoryStore::all_records_for_testing that exports all records without the response-size trimming that records() applies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Direct callers that need the complete set of records to all_records_for_testing instead, to prevent a future records(None)-for- everything mistake that would silently drop records. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the completed one-time migration scaffolding from the legacy CanisterLog store to LogMemoryStore, and makes LogMemoryStore the unconditional canister-log storage mechanism across execution, checkpointing, and tests.
Changes:
- Removed the
log_memory_store_featureflag and all migration-related state/metrics (per-canister migrated flag, subnetlogs_migrated, scheduler migration path). - Removed checkpoint/state serialization of legacy canister-log proto fields and reserved their protobuf tags/names.
- Updated test/bench helpers and APIs to read log records directly from
LogMemoryStore(including addingLogMemoryStore::all_records_for_testing).
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rs/test_utilities/execution_environment/src/lib.rs | Removes builder helpers that toggled the log-memory-store feature flag. |
| rs/state_manager/src/tip.rs | Stops serializing legacy canister-log fields into canister-state bits. |
| rs/state_manager/src/checkpoint.rs | Drops legacy canister-log fields and migration invariant checks during checkpoint load. |
| rs/state_machine_tests/src/lib.rs | Replaces canister_log() helper with record/bytes-used helpers reading from LogMemoryStore. |
| rs/state_layout/src/state_layout/tests.rs | Updates state-layout tests to stop depending on legacy canister-log fields/flags. |
| rs/state_layout/src/state_layout/proto.rs | Removes protobuf (de)serialization of legacy canister-log fields in CanisterStateBits. |
| rs/state_layout/src/state_layout.rs | Removes legacy canister-log fields from CanisterStateBits struct. |
| rs/replicated_state/src/metrics.rs | Makes canister-log metrics read from LogMemoryStore unconditionally. |
| rs/replicated_state/src/metadata_state/proto.rs | Removes transient subnet-level logs_migrated initialization during proto load. |
| rs/replicated_state/src/metadata_state.rs | Removes transient subnet-level logs_migrated field and associated wiring. |
| rs/replicated_state/src/canister_state/system_state/log_memory_store/tests/log_memory_store.rs | Updates LogMemoryStore tests to the new constructor/checkpoint API without migration flag. |
| rs/replicated_state/src/canister_state/system_state/log_memory_store/tests/canister_lifecycle.rs | Updates lifecycle tests to construct LogMemoryStore without feature flag. |
| rs/replicated_state/src/canister_state/system_state/log_memory_store/mod.rs | Removes per-canister migration flag; adds all_records_for_testing; adds Default. |
| rs/replicated_state/src/canister_state/system_state.rs | Removes SystemState.canister_log field and all feature-flag plumbing; uses LogMemoryStore only. |
| rs/replicated_state/src/canister_state.rs | Updates log-related APIs to operate solely on LogMemoryStore. |
| rs/replicated_state/benches/log_memory_store.rs | Updates benchmark setup for new LogMemoryStore::new() signature. |
| rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs | Removes generated protobuf fields for legacy canister-log storage/migration. |
| rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto | Reserves removed legacy canister-log tags/names (43, 44, 66). |
| rs/migration_canister/tests/tests.rs | Removes conditional test logic tied to log-memory-store feature enablement. |
| rs/ledger_suite/icp/ledger/tests/tests.rs | Updates tests to use StateMachine::canister_log_records. |
| rs/execution_environment/tests/memory_matrix.rs | Removes early-returns conditioned on legacy feature flag. |
| rs/execution_environment/tests/canister_logging.rs | Updates logging tests to assume LogMemoryStore always enabled; removes flag-specific tests. |
| rs/execution_environment/tests/backtraces.rs | Updates log inspection to use canister_log_records() results. |
| rs/execution_environment/src/scheduler/test_utilities.rs | Updates scheduler test builder construction after removing feature-flag plumbing. |
| rs/execution_environment/src/scheduler/scheduler_metrics.rs | Removes the migration-duration histogram metric. |
| rs/execution_environment/src/scheduler.rs | Removes subnet-level migration execution path and legacy canister-log filtering helpers/tests. |
| rs/execution_environment/src/query_handler.rs | Switches fetch_canister_logs_response call to the new signature (no feature flag). |
| rs/execution_environment/src/lib.rs | Removes passing the removed feature flag through execution services setup. |
| rs/execution_environment/src/execution/install_code.rs | Updates install-code error plumbing and log handling to use LogMemoryStore only. |
| rs/execution_environment/src/execution_environment.rs | Updates canister-log fetch call sites after removing feature flag parameter. |
| rs/execution_environment/src/canister_manager/types.rs | Removes the feature-flag field from the canister-manager config struct. |
| rs/execution_environment/src/canister_manager/tests.rs | Removes feature-flag-dependent assertions; updates builders accordingly. |
| rs/execution_environment/src/canister_manager.rs | Makes log-memory-limit resize logic unconditional (since LogMemoryStore is always active). |
| rs/execution_environment/src/canister_logs.rs | Removes legacy-store fallback and always reads canister logs from LogMemoryStore. |
| rs/execution_environment/benches/management_canister/canister_logging.rs | Updates management-canister log-fetch benchmarks after removing feature flag. |
| rs/ethereum/cketh/test_utils/src/lib.rs | Updates log extraction helper to use canister_log_records(). |
| rs/embedders/src/wasmtime_embedder/tests.rs | Updates system-state construction call sites after removing feature flag parameter. |
| rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs | Routes delta log appends through LogMemoryStore unconditionally. |
| rs/config/src/execution_environment.rs | Removes the flag/constants; makes test defaults unconditional for log-memory-store. |
| rs/bitcoin/ckbtc/minter/tests/tests.rs | Updates minter log printing helper to use canister_log_records(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mraszyk
commented
Jul 10, 2026
|
✅ No security or compliance issues detected. Reviewed everything up to 9803bf1. Security Overview
Detected Code ChangesThe diff is too large to display a summary of code changes. |
|
✅ No security or compliance issues detected. Reviewed everything up to 9803bf1. Security Overview
Detected Code ChangesThe diff is too large to display a summary of code changes. |
|
✅ No security or compliance issues detected. Reviewed everything up to 9803bf1. Security Overview
Detected Code ChangesThe diff is too large to display a summary of code changes. |
mbjorkqvist
approved these changes
Jul 10, 2026
alin-at-dfinity
approved these changes
Jul 13, 2026
eichhorl
reviewed
Jul 13, 2026
The helper no longer takes the canister log; it clones the (cheap, persistent-PageMap-backed) log memory store. Rename it accordingly, switch it to &self, and update the doc comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
eichhorl
approved these changes
Jul 13, 2026
pull Bot
pushed a commit
to bit-cook/ic
that referenced
this pull request
Jul 13, 2026
… store feature flag (dfinity#10714)" (dfinity#10749) This reverts commit 5b0f6ee. Because canister logs would be lost if we had to downgrade the replica version again.
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 one-time migration from the legacy
CanisterLogstore to theLogMemoryStorehas completed on all subnets, so the migration scaffolding and the legacy store can be removed.log_memory_store_featureflag (and theLOG_MEMORY_STORE_FEATURE/_ENABLEDconsts); the log memory store is now always enabled.SystemState.canister_logfield and theCanisterStatelog wrappers now operate solely onlog_memory_store.migratedflag (is_migrated/set_migrated/clear_migrated), the subnet-levellogs_migratedmetadata, the scheduler migration code, and the migration-duration metric. Reads, writes and metrics now uselog_memory_storeunconditionally.canister_log_records,next_canister_log_record_idxandlog_memory_store_migrated; the corresponding proto fields (43, 44, 66) are marked reserved.StateMachine::canister_log testhelper (which rebuilt aCanisterLogaggregate) withcanister_log_recordsandcanister_log_bytes_used, reading directly fromLogMemoryStore. Add a test-onlyLogMemoryStore::all_records_for_testingthat returns all records without the response-size trimming thatrecords()applies, and migrate all callers.The
CanisterLogtype and the transientSystemStateModificationsdelta log buffer are retained.