From 178fb9795c5d6642bde9aa714050916ca70bd240 Mon Sep 17 00:00:00 2001 From: Alexandra Zapuc Date: Wed, 14 Feb 2024 13:45:02 +0000 Subject: [PATCH] feat: EXC-1527: Add canister snapshots to `ReplicatedState` --- rs/http_endpoints/public/src/read_state/canister.rs | 5 ++++- .../public/src/state_reader_executor.rs | 5 ++++- rs/http_endpoints/public/tests/common/mod.rs | 6 +++++- rs/ingress_manager/benches/handle_ingress.rs | 5 ++++- rs/replicated_state/src/canister_snapshots.rs | 5 +++++ rs/replicated_state/src/replicated_state.rs | 12 ++++++++++++ rs/state_manager/src/checkpoint.rs | 12 ++++++++++-- rs/state_manager/src/tip.rs | 3 +++ 8 files changed, 47 insertions(+), 6 deletions(-) diff --git a/rs/http_endpoints/public/src/read_state/canister.rs b/rs/http_endpoints/public/src/read_state/canister.rs index 2a2c4f46505..f7d5a5f0c33 100644 --- a/rs/http_endpoints/public/src/read_state/canister.rs +++ b/rs/http_endpoints/public/src/read_state/canister.rs @@ -430,7 +430,9 @@ mod test { use hyper::StatusCode; use ic_crypto_tree_hash::{Digest, Label, MixedHashTree, Path}; use ic_registry_subnet_type::SubnetType; - use ic_replicated_state::{CanisterQueues, ReplicatedState, SystemMetadata}; + use ic_replicated_state::{ + canister_snapshots::CanisterSnapshots, CanisterQueues, ReplicatedState, SystemMetadata, + }; use ic_test_utilities::{ state::insert_dummy_canister, types::ids::{canister_test_id, subnet_test_id, user_test_id}, @@ -558,6 +560,7 @@ mod test { metadata, CanisterQueues::default(), RawQueryStats::default(), + CanisterSnapshots::default(), ); assert_eq!( verify_paths( diff --git a/rs/http_endpoints/public/src/state_reader_executor.rs b/rs/http_endpoints/public/src/state_reader_executor.rs index 0ebbd483096..93bd2b73021 100644 --- a/rs/http_endpoints/public/src/state_reader_executor.rs +++ b/rs/http_endpoints/public/src/state_reader_executor.rs @@ -80,7 +80,9 @@ mod tests { use ic_crypto_tree_hash::{flatmap, Label, LabeledTree}; use ic_interfaces_state_manager_mocks::MockStateManager; use ic_registry_subnet_type::SubnetType; - use ic_replicated_state::{CanisterQueues, ReplicatedState, SystemMetadata}; + use ic_replicated_state::{ + canister_snapshots::CanisterSnapshots, CanisterQueues, ReplicatedState, SystemMetadata, + }; use ic_test_utilities::{state::ReplicatedStateBuilder, types::ids::subnet_test_id}; use ic_test_utilities_time::mock_time; use ic_types::{ @@ -111,6 +113,7 @@ mod tests { metadata, CanisterQueues::default(), RawQueryStats::default(), + CanisterSnapshots::default(), )), ) }); diff --git a/rs/http_endpoints/public/tests/common/mod.rs b/rs/http_endpoints/public/tests/common/mod.rs index 86647abbc17..2d23fe1f50f 100644 --- a/rs/http_endpoints/public/tests/common/mod.rs +++ b/rs/http_endpoints/public/tests/common/mod.rs @@ -34,7 +34,10 @@ use ic_registry_keys::{ use ic_registry_provisional_whitelist::ProvisionalWhitelist; use ic_registry_routing_table::{CanisterMigrations, RoutingTable}; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::{CanisterQueues, NetworkTopology, ReplicatedState, SystemMetadata}; +use ic_replicated_state::{ + canister_snapshots::CanisterSnapshots, CanisterQueues, NetworkTopology, ReplicatedState, + SystemMetadata, +}; use ic_test_utilities::{ crypto::{temp_crypto_component_with_fake_registry, CryptoReturningOk}, state::ReplicatedStateBuilder, @@ -211,6 +214,7 @@ pub fn default_get_latest_state() -> Labeled> { metadata, CanisterQueues::default(), RawQueryStats::default(), + CanisterSnapshots::default(), )), ) } diff --git a/rs/ingress_manager/benches/handle_ingress.rs b/rs/ingress_manager/benches/handle_ingress.rs index ee0cecfcc96..53abe91296f 100644 --- a/rs/ingress_manager/benches/handle_ingress.rs +++ b/rs/ingress_manager/benches/handle_ingress.rs @@ -32,7 +32,9 @@ use ic_registry_client::client::RegistryClientImpl; use ic_registry_keys::make_subnet_record_key; use ic_registry_proto_data_provider::ProtoRegistryDataProvider; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::{CanisterQueues, ReplicatedState, SystemMetadata}; +use ic_replicated_state::{ + canister_snapshots::CanisterSnapshots, CanisterQueues, ReplicatedState, SystemMetadata, +}; use ic_test_utilities::{ crypto::temp_crypto_component_with_fake_registry, cycles_account_manager::CyclesAccountManagerBuilder, @@ -189,6 +191,7 @@ where metadata, CanisterQueues::default(), RawQueryStats::default(), + CanisterSnapshots::default(), )), ) }); diff --git a/rs/replicated_state/src/canister_snapshots.rs b/rs/replicated_state/src/canister_snapshots.rs index 8b5a9b16dde..d213b9738ba 100644 --- a/rs/replicated_state/src/canister_snapshots.rs +++ b/rs/replicated_state/src/canister_snapshots.rs @@ -80,6 +80,11 @@ impl CanisterSnapshots { pub fn take_unflushed_changes(&mut self) -> Vec { std::mem::take(&mut self.unflushed_changes) } + + /// Returns true if unflushed changes list is empty. + pub fn is_unflushed_changes_empty(&self) -> bool { + self.unflushed_changes.is_empty() + } } /// Contains all information related to a canister snapshot. diff --git a/rs/replicated_state/src/replicated_state.rs b/rs/replicated_state/src/replicated_state.rs index 013658bd7b9..c59647e443e 100644 --- a/rs/replicated_state/src/replicated_state.rs +++ b/rs/replicated_state/src/replicated_state.rs @@ -3,6 +3,7 @@ use super::{ metadata_state::{IngressHistoryState, Stream, Streams, SystemMetadata}, }; use crate::{ + canister_snapshots::CanisterSnapshots, canister_state::queues::CanisterQueuesLoopDetector, canister_state::system_state::{push_input, CanisterOutputQueuesIterator}, metadata_state::{subnet_call_context_manager::SignWithEcdsaContext, StreamMap}, @@ -406,6 +407,9 @@ pub struct ReplicatedState { /// Temporary query stats received during the current epoch. /// Reset during the start of each epoch. pub epoch_query_stats: RawQueryStats, + + /// Manages the canister snapshots. + pub canister_snapshots: CanisterSnapshots, } impl ReplicatedState { @@ -417,6 +421,7 @@ impl ReplicatedState { subnet_queues: CanisterQueues::default(), consensus_queue: Vec::new(), epoch_query_stats: RawQueryStats::default(), + canister_snapshots: CanisterSnapshots::default(), } } @@ -426,6 +431,7 @@ impl ReplicatedState { metadata: SystemMetadata, subnet_queues: CanisterQueues, epoch_query_stats: RawQueryStats, + canister_snapshots: CanisterSnapshots, ) -> Self { let mut res = Self { canister_states, @@ -433,6 +439,7 @@ impl ReplicatedState { subnet_queues, consensus_queue: Vec::new(), epoch_query_stats, + canister_snapshots, }; res.update_stream_responses_size_bytes(); res @@ -923,6 +930,7 @@ impl ReplicatedState { mut subnet_queues, consensus_queue, epoch_query_stats: _, + canister_snapshots, } = self; // Consensus queue is always empty at the end of the round. @@ -958,6 +966,7 @@ impl ReplicatedState { subnet_queues, consensus_queue, epoch_query_stats: RawQueryStats::default(), // Don't preserve query stats during subnet splitting. + canister_snapshots, }) } @@ -980,6 +989,7 @@ impl ReplicatedState { ref mut subnet_queues, consensus_queue: _, epoch_query_stats: _, + canister_snapshots: _, } = self; // Reset query stats after subnet split @@ -1140,6 +1150,8 @@ pub mod testing { subnet_queues: Default::default(), consensus_queue: Default::default(), epoch_query_stats: Default::default(), + // TODO(EXC-1527): Handle canister snapshots during a subnet split. + canister_snapshots: CanisterSnapshots::default(), }; } } diff --git a/rs/state_manager/src/checkpoint.rs b/rs/state_manager/src/checkpoint.rs index b5d2d3c8932..b9736238131 100644 --- a/rs/state_manager/src/checkpoint.rs +++ b/rs/state_manager/src/checkpoint.rs @@ -7,6 +7,7 @@ use ic_base_types::{subnet_id_try_from_protobuf, CanisterId}; use ic_config::flag_status::FlagStatus; use ic_logger::error; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::canister_snapshots::CanisterSnapshots; use ic_replicated_state::page_map::PageAllocatorFileDescriptor; use ic_replicated_state::{ canister_state::execution_state::WasmBinary, page_map::PageMap, CanisterMetrics, CanisterState, @@ -258,8 +259,15 @@ pub fn load_checkpoint( canister_states }; - let state = - ReplicatedState::new_from_checkpoint(canister_states, metadata, subnet_queues, query_stats); + // TODO(EXC-1539): Load canister snapshots from checkpoint. + let canister_snapshots = CanisterSnapshots::default(); + let state = ReplicatedState::new_from_checkpoint( + canister_states, + metadata, + subnet_queues, + query_stats, + canister_snapshots, + ); Ok(state) } diff --git a/rs/state_manager/src/tip.rs b/rs/state_manager/src/tip.rs index 59bd7cb0aa9..c7b535a4908 100644 --- a/rs/state_manager/src/tip.rs +++ b/rs/state_manager/src/tip.rs @@ -788,6 +788,9 @@ fn serialize_to_tip( metrics: &StorageMetrics, lsmt_storage: FlagStatus, ) -> Result<(), CheckpointError> { + //TODO(MR-530): Implement serializing canister snapshots. + debug_assert!(state.canister_snapshots.is_unflushed_changes_empty()); + // Serialize ingress history separately. The `SystemMetadata` proto does not // encode it. //