From 0a840fbdbdd65d2c41e8f1daf0efc8fa7b87b4e7 Mon Sep 17 00:00:00 2001 From: Vlad Lazar Date: Thu, 10 Oct 2024 19:28:01 +0200 Subject: [PATCH] pageserver: purge read path caching support We now only store indices in the page cache. This commit removes any caching support from the read path. --- pageserver/src/tenant/storage_layer.rs | 38 ------------------- .../src/tenant/storage_layer/delta_layer.rs | 14 +------ .../tenant/storage_layer/inmemory_layer.rs | 13 ++----- pageserver/src/tenant/timeline.rs | 2 - 4 files changed, 4 insertions(+), 63 deletions(-) diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index e75076f158..fba8424153 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -135,12 +135,6 @@ impl VectoredValueReconstructState { Ok(to) } - - fn get_cached_lsn(&self) -> Option { - // self.img.as_ref().map(|img| img.0) - // TODO: rip this out - Some(Lsn(0)) - } } /// Bag of data accumulated during a vectored get.. @@ -239,29 +233,6 @@ impl ValuesReconstructState { self.layers_visited } - /// This function is called after reading a keyspace from a layer. - /// It checks if the read path has now moved past the cached Lsn for any keys. - /// - /// Implementation note: We intentionally iterate over the keys for which we've - /// already collected some reconstruct data. This avoids scaling complexity with - /// the size of the search space. - pub(crate) fn on_lsn_advanced(&mut self, keyspace: &KeySpace, advanced_to: Lsn) { - for (key, value) in self.keys.iter_mut() { - if !keyspace.contains(key) { - continue; - } - - if let Ok(state) = value { - if state.situation != ValueReconstructSituation::Complete - && state.get_cached_lsn() >= Some(advanced_to) - { - state.situation = ValueReconstructSituation::Complete; - self.keys_done.add_key(*key); - } - } - } - } - /// On hitting image layer, we can mark all keys in this range as done, because /// if the image layer does not contain a key, it is deleted/never added. pub(crate) fn on_image_layer_visited(&mut self, key_range: &Range) { @@ -307,15 +278,6 @@ impl ValuesReconstructState { } } - /// Returns the Lsn at which this key is cached if one exists. - /// The read path should go no further than this Lsn for the given key. - pub(crate) fn get_cached_lsn(&self, key: &Key) -> Option { - self.keys - .get(key) - .and_then(|k| k.as_ref().ok()) - .and_then(|state| state.get_cached_lsn()) - } - /// Returns the key space describing the keys that have /// been marked as completed since the last call to this function. /// Returns individual keys done, and the image layer coverage. diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index 03179414de..7eb94d8b88 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -47,7 +47,6 @@ use crate::virtual_file::{self, MaybeFatalIo, VirtualFile}; use crate::TEMP_FILE_SUFFIX; use crate::{DELTA_FILE_MAGIC, STORAGE_FORMAT_VERSION}; use anyhow::{bail, ensure, Context, Result}; -use bytes::Bytes; use camino::{Utf8Path, Utf8PathBuf}; use futures::StreamExt; use itertools::Itertools; @@ -840,8 +839,6 @@ impl DeltaLayerInner { // Look up the keys in the provided keyspace and update // the reconstruct state with whatever is found. // - // If the key is cached, go no further than the cached Lsn. - // // Currently, the index is visited for each range, but this // can be further optimised to visit the index only once. pub(super) async fn get_values_reconstruct_data( @@ -873,7 +870,6 @@ impl DeltaLayerInner { data_end_offset, index_reader, planner, - reconstruct_state, ctx, ) .await @@ -882,8 +878,6 @@ impl DeltaLayerInner { self.do_reads_and_update_state(reads, reconstruct_state, ctx) .await; - reconstruct_state.on_lsn_advanced(&keyspace, lsn_range.start); - Ok(()) } @@ -893,7 +887,6 @@ impl DeltaLayerInner { data_end_offset: u64, index_reader: DiskBtreeReader, mut planner: VectoredReadPlanner, - reconstruct_state: &mut ValuesReconstructState, ctx: &RequestContext, ) -> anyhow::Result> where @@ -920,10 +913,9 @@ impl DeltaLayerInner { assert!(key >= range.start); let outside_lsn_range = !lsn_range.contains(&lsn); - let below_cached_lsn = reconstruct_state.get_cached_lsn(&key) >= Some(lsn); let flag = { - if outside_lsn_range || below_cached_lsn { + if outside_lsn_range { BlobFlag::Ignore } else if blob_ref.will_init() { BlobFlag::ReplaceAll @@ -1660,7 +1652,6 @@ pub(crate) mod test { .expect("In memory disk finish should never fail"); let reader = DiskBtreeReader::<_, DELTA_KEY_SIZE>::new(0, root_offset, disk); let planner = VectoredReadPlanner::new(100); - let mut reconstruct_state = ValuesReconstructState::new(); let ctx = RequestContext::new(TaskKind::UnitTest, DownloadBehavior::Error); let keyspace = KeySpace { @@ -1678,7 +1669,6 @@ pub(crate) mod test { disk_offset, reader, planner, - &mut reconstruct_state, &ctx, ) .await @@ -1922,7 +1912,6 @@ pub(crate) mod test { ); let planner = VectoredReadPlanner::new(constants::MAX_VECTORED_READ_BYTES); - let mut reconstruct_state = ValuesReconstructState::new(); let keyspace = pick_random_keyspace(rng, &entries_meta.key_range); let data_end_offset = inner.index_start_blk as u64 * PAGE_SZ as u64; @@ -1932,7 +1921,6 @@ pub(crate) mod test { data_end_offset, index_reader, planner, - &mut reconstruct_state, &ctx, ) .await?; diff --git a/pageserver/src/tenant/storage_layer/inmemory_layer.rs b/pageserver/src/tenant/storage_layer/inmemory_layer.rs index cc7284d5bb..ac2a8ff4a3 100644 --- a/pageserver/src/tenant/storage_layer/inmemory_layer.rs +++ b/pageserver/src/tenant/storage_layer/inmemory_layer.rs @@ -414,8 +414,6 @@ impl InMemoryLayer { // Look up the keys in the provided keyspace and update // the reconstruct state with whatever is found. - // - // If the key is cached, go no further than the cached Lsn. pub(crate) async fn get_values_reconstruct_data( &self, keyspace: KeySpace, @@ -439,18 +437,15 @@ impl InMemoryLayer { tokio::sync::oneshot::Sender>, > = Default::default(); + let lsn_range = self.start_lsn..end_lsn; + for range in keyspace.ranges.iter() { for (key, vec_map) in inner .index .range(range.start.to_compact()..range.end.to_compact()) { let key = Key::from_compact(*key); - let lsn_range = match reconstruct_state.get_cached_lsn(&key) { - Some(cached_lsn) => (cached_lsn + 1)..end_lsn, - None => self.start_lsn..end_lsn, - }; - - let slice = vec_map.slice_range(lsn_range); + let slice = vec_map.slice_range(lsn_range.clone()); for (entry_lsn, index_entry) in slice.iter().rev() { let IndexEntryUnpacked { @@ -512,8 +507,6 @@ impl InMemoryLayer { assert!(senders.is_empty()); }); - reconstruct_state.on_lsn_advanced(&keyspace, self.start_lsn); - Ok(()) } } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 6db1107bb2..6babf47620 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -938,8 +938,6 @@ impl Timeline { ranges: vec![key..key.next()], }; - // Initialise the reconstruct state for the key with the cache - // entry returned above. let mut reconstruct_state = ValuesReconstructState::new(); let vectored_res = self