diff --git a/pageserver/src/tenant/layer_map.rs b/pageserver/src/tenant/layer_map.rs index 572a6a4533..112c16867b 100644 --- a/pageserver/src/tenant/layer_map.rs +++ b/pageserver/src/tenant/layer_map.rs @@ -249,77 +249,55 @@ impl LayerMap { /// 'open' and 'frozen' layers! /// pub fn search(&self, key: Key, end_lsn: Lsn) -> Option { - self.search_incremental(key, end_lsn).map(|(x, _)| x) + self.search_incremental(key, end_lsn, false) } pub fn search_incremental( &self, key: Key, end_lsn: Lsn, - ) -> Option<(SearchResult, Option)> { + exclude_image: bool, + ) -> Option { let version = self.historic.get().unwrap().get_version(end_lsn.0 - 1)?; let latest_delta = version.delta_coverage.query(key.to_i128()); - let latest_image = version.image_coverage.query(key.to_i128()); + let latest_image = if exclude_image { + None + } else { + version.image_coverage.query(key.to_i128()) + }; match (latest_delta, latest_image) { (None, None) => None, (None, Some(image)) => { let lsn_floor = image.get_lsn_range().start; - Some(( - SearchResult { - layer: image, - lsn_floor, - }, - None, - )) + Some(SearchResult { + layer: image, + lsn_floor, + }) } (Some(delta), None) => { let lsn_floor = delta.get_lsn_range().start; - Some(( - SearchResult { - layer: delta, - lsn_floor, - }, - None, - )) + Some(SearchResult { + layer: delta, + lsn_floor, + }) } (Some(delta), Some(image)) => { let img_lsn = image.get_lsn_range().start; let image_is_newer = image.get_lsn_range().end >= delta.get_lsn_range().end; let image_exact_match = img_lsn + 1 == end_lsn; - if image_is_newer || image_exact_match { - if image.get_lsn_range().end == delta.get_lsn_range().end { - // incremental, image lsn N, if it does not contain the image, we should start with - // delta lsn N+1 instead of N. - Some(( - SearchResult { - layer: image, - lsn_floor: img_lsn, - }, - Some(SearchResult { - lsn_floor: delta.get_lsn_range().start, - layer: delta, - }), - )) - } else { - Some(( - SearchResult { - layer: image, - lsn_floor: img_lsn, - }, - None, - )) - } + if image_is_newer { + Some(SearchResult { + layer: image, + lsn_floor: img_lsn, + }) } else { let lsn_floor = std::cmp::max(delta.get_lsn_range().start, image.get_lsn_range().start + 1); - Some(( - SearchResult { - layer: delta, - lsn_floor, - }, - None, - )) + Some(SearchResult { + layer: delta, + lsn_floor, + }) } } } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 8fe922fa86..a70cec2759 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -2434,7 +2434,11 @@ impl Timeline { let mut result = ValueReconstructResult::Continue; let mut cont_lsn = Lsn(request_lsn.0 + 1); + let mut last_round_image = false; + 'outer: loop { + let exclude_image_this_round = last_round_image; + last_round_image = false; // The function should have updated 'state' //info!("CALLED for {} at {}: {:?} with {} records, cached {}", key, cont_lsn, result, reconstruct_state.records.len(), cached_lsn); match result { @@ -2599,8 +2603,8 @@ impl Timeline { } } - if let Some((SearchResult { lsn_floor, layer }, next)) = - layers.search_incremental(key, cont_lsn) + if let Some(SearchResult { lsn_floor, layer }) = + layers.search_incremental(key, cont_lsn, exclude_image_this_round) { let layer = timeline.lcache.get_from_desc(&layer); // If it's a remote layer, download it and retry. @@ -2627,46 +2631,8 @@ impl Timeline { if !layer.layer_desc().is_delta && matches!(result, ValueReconstructResult::Continue) { - let old_layer = layer.clone(); - // if is incremental image layer and not found, try again with delta layer - if let Some(SearchResult { lsn_floor, layer }) = next { - traversal_path.push(( - result, - cont_lsn, - Box::new({ - let layer = Arc::clone(&old_layer); - move || layer.traversal_id() - }), - )); - - // HACK: no remote storage for now, safely get and downcast - let layer = timeline.lcache.get_from_desc(&layer); - - // Get all the data needed to reconstruct the page version from this layer. - // But if we have an older cached page image, no need to go past that. - let lsn_floor = max(cached_lsn + 1, lsn_floor); - result = match layer.get_value_reconstruct_data( - key, - lsn_floor..cont_lsn, - reconstruct_state, - ctx, - ) { - Ok(result) => result, - Err(e) => return Err(PageReconstructError::from(e)), - }; - - cont_lsn = lsn_floor; - *read_count += 2; - traversal_path.push(( - result, - cont_lsn, - Box::new({ - let layer = Arc::clone(&layer); - move || layer.traversal_id() - }), - )); - continue 'outer; - } + last_round_image = true; + continue 'outer; }; cont_lsn = lsn_floor; @@ -4166,7 +4132,7 @@ impl Timeline { } } - const ENABLE_TRIVIAL_MOVE: bool = false; + const ENABLE_TRIVIAL_MOVE: bool = true; if !ENABLE_TRIVIAL_MOVE { deltas_to_compact_layers.extend(std::mem::take(&mut trivial_move_layers));