diff --git a/pageserver/src/tenant/layer_map.rs b/pageserver/src/tenant/layer_map.rs index 0787c80708..7b76199190 100644 --- a/pageserver/src/tenant/layer_map.rs +++ b/pageserver/src/tenant/layer_map.rs @@ -51,7 +51,7 @@ use crate::keyspace::KeyPartitioning; use crate::repository::Key; use crate::tenant::storage_layer::InMemoryLayer; use crate::tenant::storage_layer::Layer; -use anyhow::Result; +use anyhow::{bail, Result}; use std::collections::VecDeque; use std::ops::Range; use std::sync::Arc; @@ -276,16 +276,22 @@ where /// pub(self) fn insert_historic_noflush(&mut self, layer: Arc) -> anyhow::Result<()> { let key = historic_layer_coverage::LayerKey::from(&*layer); - if self.historic.contains(&key) { - error!( - "Attempt to insert duplicate layer {} in layer map", - layer.short_id() - ); - } else { - self.historic.insert(key, Arc::clone(&layer)); + match self.historic.replace(&key, Arc::clone(&layer), |existing| { + !Self::compare_arced_layers(existing, &layer) + }) { + Replacement::Replaced { .. } => { + if Self::is_l0(&layer) { + bail!("Duplicate L0 layer {}", layer.short_id()); + } + warn!("Replace duplicate layer {} in layer map", layer.short_id()); + } + Replacement::Unexpected(_) => bail!("Replace layer with itslef is prohibited"), + Replacement::NotFound | Replacement::RemovalBuffered => { + self.historic.insert(key, Arc::clone(&layer)); - if Self::is_l0(&layer) { - self.l0_delta_layers.push(layer); + if Self::is_l0(&layer) { + self.l0_delta_layers.push(layer); + } } } Ok(()) diff --git a/pageserver/src/tenant/layer_map/historic_layer_coverage.rs b/pageserver/src/tenant/layer_map/historic_layer_coverage.rs index 1fdcd5e5a4..b63c361314 100644 --- a/pageserver/src/tenant/layer_map/historic_layer_coverage.rs +++ b/pageserver/src/tenant/layer_map/historic_layer_coverage.rs @@ -417,14 +417,6 @@ impl BufferedHistoricLayerCoverage { } } - pub fn contains(&self, layer_key: &LayerKey) -> bool { - match self.buffer.get(layer_key) { - Some(None) => false, // layer remove was buffered - Some(_) => true, // layer insert was buffered - None => self.layers.contains_key(layer_key), // no buffered ops for this layer - } - } - pub fn insert(&mut self, layer_key: LayerKey, value: Value) { self.buffer.insert(layer_key, Some(value)); } diff --git a/test_runner/regress/test_duplicate_layers.py b/test_runner/regress/test_duplicate_layers.py index 4ff4e1e288..77d79cfcc0 100644 --- a/test_runner/regress/test_duplicate_layers.py +++ b/test_runner/regress/test_duplicate_layers.py @@ -16,7 +16,7 @@ def test_duplicate_layers(neon_env_builder: NeonEnvBuilder, pg_bin: PgBin): # These warnings are expected, when the pageserver is restarted abruptly env.pageserver.allowed_errors.append(".*found future image layer.*") env.pageserver.allowed_errors.append(".*found future delta layer.*") - env.pageserver.allowed_errors.append(".*Attempt to insert duplicate layer.*") + env.pageserver.allowed_errors.append(".*duplicate layer.*") pageserver_http = env.pageserver.http_client()