diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index 3f69017160..985b480a76 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -1135,18 +1135,29 @@ mod tests { client.init_upload_queue_for_empty_remote(&metadata)?; // Create a couple of dummy files, schedule upload for them - let content_foo = dummy_contents("foo"); - let content_bar = dummy_contents("bar"); - std::fs::write(timeline_path.join("foo"), &content_foo)?; - std::fs::write(timeline_path.join("bar"), &content_bar)?; + let layer_file_name_1: LayerFileName = "000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D8-00000000016B5A51".parse().unwrap(); + let layer_file_name_2: LayerFileName = "000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D9-00000000016B5A52".parse().unwrap(); + let layer_file_name_3: LayerFileName = "000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59DA-00000000016B5A53".parse().unwrap(); + let content_1 = dummy_contents("foo"); + let content_2 = dummy_contents("bar"); + let content_3 = dummy_contents("baz"); + std::fs::write( + timeline_path.join(layer_file_name_1.file_name()), + &content_1, + )?; + std::fs::write( + timeline_path.join(layer_file_name_2.file_name()), + &content_2, + )?; + std::fs::write(timeline_path.join(layer_file_name_3.file_name()), content_3)?; client.schedule_layer_file_upload( - &LayerFileName::Test("foo".to_owned()), - &LayerFileMetadata::new(content_foo.len() as u64), + &layer_file_name_1, + &LayerFileMetadata::new(content_1.len() as u64), )?; client.schedule_layer_file_upload( - &LayerFileName::Test("bar".to_owned()), - &LayerFileMetadata::new(content_bar.len() as u64), + &layer_file_name_2, + &LayerFileMetadata::new(content_2.len() as u64), )?; // Check that they are started immediately, not queued @@ -1183,7 +1194,13 @@ mod tests { // Download back the index.json, and check that the list of files is correct let index_part = runtime.block_on(client.download_index_file())?; - assert_file_list(&index_part.timeline_layers, &["foo", "bar"]); + assert_file_list( + &index_part.timeline_layers, + &[ + &layer_file_name_1.file_name(), + &layer_file_name_2.file_name(), + ], + ); let downloaded_metadata = index_part.parse_metadata()?; assert_eq!(downloaded_metadata, metadata); @@ -1191,10 +1208,10 @@ mod tests { let content_baz = dummy_contents("baz"); std::fs::write(timeline_path.join("baz"), &content_baz)?; client.schedule_layer_file_upload( - &LayerFileName::Test("baz".to_owned()), + &layer_file_name_3, &LayerFileMetadata::new(content_baz.len() as u64), )?; - client.schedule_layer_file_deletion(&[LayerFileName::Test("foo".to_owned())])?; + client.schedule_layer_file_deletion(&[layer_file_name_1.clone()])?; { let mut guard = client.upload_queue.lock().unwrap(); let upload_queue = guard.initialized_mut().unwrap(); @@ -1206,12 +1223,26 @@ mod tests { assert!(upload_queue.num_inprogress_deletions == 0); assert!(upload_queue.latest_files_changes_since_metadata_upload_scheduled == 0); } - assert_remote_files(&["foo", "bar", "index_part.json"], &remote_timeline_dir); + assert_remote_files( + &[ + &layer_file_name_1.file_name(), + &layer_file_name_2.file_name(), + "index_part.json", + ], + &remote_timeline_dir, + ); // Finish them runtime.block_on(client.wait_completion())?; - assert_remote_files(&["bar", "baz", "index_part.json"], &remote_timeline_dir); + assert_remote_files( + &[ + &layer_file_name_2.file_name(), + &layer_file_name_3.file_name(), + "index_part.json", + ], + &remote_timeline_dir, + ); Ok(()) } diff --git a/pageserver/src/tenant/remote_timeline_client/index.rs b/pageserver/src/tenant/remote_timeline_client/index.rs index c199b7e10b..420edae6cd 100644 --- a/pageserver/src/tenant/remote_timeline_client/index.rs +++ b/pageserver/src/tenant/remote_timeline_client/index.rs @@ -8,7 +8,8 @@ use serde::{Deserialize, Serialize}; use serde_with::{serde_as, DisplayFromStr}; use tracing::warn; -use crate::tenant::{metadata::TimelineMetadata, storage_layer::LayerFileName}; +use crate::tenant::metadata::TimelineMetadata; +use crate::tenant::storage_layer::LayerFileName; use utils::lsn::Lsn; @@ -274,7 +275,7 @@ mod tests { "timeline_layers":["000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9"], "layer_metadata":{ "000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9": { "file_size": 25600000 }, - "LAYER_FILE_NAME::test/not_a_real_layer_but_adding_coverage": { "file_size": 9007199254741001 } + "000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D8-00000000016B5A51": { "file_size": 9007199254741001 } }, "disk_consistent_lsn":"0/16960E8", "metadata_bytes":[113,11,159,210,0,54,0,4,0,0,0,0,1,105,96,232,1,0,0,0,0,1,105,96,112,0,0,0,0,0,0,0,0,0,0,0,0,0,1,105,96,112,0,0,0,0,1,105,96,112,0,0,0,14,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0] @@ -288,7 +289,7 @@ mod tests { ("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9".parse().unwrap(), IndexLayerMetadata { file_size: Some(25600000), }), - (LayerFileName::new_test("not_a_real_layer_but_adding_coverage"), IndexLayerMetadata { + ("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D8-00000000016B5A51".parse().unwrap(), IndexLayerMetadata { // serde_json should always parse this but this might be a double with jq for // example. file_size: Some(9007199254741001), @@ -312,7 +313,7 @@ mod tests { "missing_layers":["This shouldn't fail deserialization"], "layer_metadata":{ "000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9": { "file_size": 25600000 }, - "LAYER_FILE_NAME::test/not_a_real_layer_but_adding_coverage": { "file_size": 9007199254741001 } + "000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D8-00000000016B5A51": { "file_size": 9007199254741001 } }, "disk_consistent_lsn":"0/16960E8", "metadata_bytes":[112,11,159,210,0,54,0,4,0,0,0,0,1,105,96,232,1,0,0,0,0,1,105,96,112,0,0,0,0,0,0,0,0,0,0,0,0,0,1,105,96,112,0,0,0,0,1,105,96,112,0,0,0,14,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0] @@ -326,7 +327,7 @@ mod tests { ("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9".parse().unwrap(), IndexLayerMetadata { file_size: Some(25600000), }), - (LayerFileName::new_test("not_a_real_layer_but_adding_coverage"), IndexLayerMetadata { + ("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D8-00000000016B5A51".parse().unwrap(), IndexLayerMetadata { // serde_json should always parse this but this might be a double with jq for // example. file_size: Some(9007199254741001), diff --git a/pageserver/src/tenant/storage_layer/filename.rs b/pageserver/src/tenant/storage_layer/filename.rs index 7ebf346ec9..e77f2b25c1 100644 --- a/pageserver/src/tenant/storage_layer/filename.rs +++ b/pageserver/src/tenant/storage_layer/filename.rs @@ -175,49 +175,32 @@ impl fmt::Display for ImageFileName { pub enum LayerFileName { Image(ImageFileName), Delta(DeltaFileName), - #[cfg(test)] - Test(String), } impl LayerFileName { pub fn file_name(&self) -> String { match self { - LayerFileName::Image(fname) => format!("{fname}"), - LayerFileName::Delta(fname) => format!("{fname}"), - #[cfg(test)] - LayerFileName::Test(fname) => fname.to_string(), + Self::Image(fname) => fname.to_string(), + Self::Delta(fname) => fname.to_string(), } } - #[cfg(test)] - pub(crate) fn new_test(name: &str) -> LayerFileName { - LayerFileName::Test(name.to_owned()) - } } impl From for LayerFileName { fn from(fname: ImageFileName) -> Self { - LayerFileName::Image(fname) + Self::Image(fname) } } impl From for LayerFileName { fn from(fname: DeltaFileName) -> Self { - LayerFileName::Delta(fname) + Self::Delta(fname) } } -// include a `/` in the name as an additional layer of robustness -// because `/` chars are not allowed in UNIX paths -#[cfg(test)] -const LAYER_FILE_NAME_TEST_PREFIX: &str = "LAYER_FILE_NAME::test/"; - impl FromStr for LayerFileName { type Err = String; fn from_str(value: &str) -> Result { - #[cfg(test)] - if let Some(value) = value.strip_prefix(LAYER_FILE_NAME_TEST_PREFIX) { - return Ok(LayerFileName::Test(value.to_owned())); - } let delta = DeltaFileName::parse_str(value); let image = ImageFileName::parse_str(value); let ok = match (delta, image) { @@ -226,8 +209,8 @@ impl FromStr for LayerFileName { "neither delta nor image layer file name: {value:?}" )) } - (Some(delta), None) => LayerFileName::Delta(delta), - (None, Some(image)) => LayerFileName::Image(image), + (Some(delta), None) => Self::Delta(delta), + (None, Some(image)) => Self::Image(image), (Some(_), Some(_)) => unreachable!(), }; Ok(ok) @@ -240,12 +223,8 @@ impl serde::Serialize for LayerFileName { S: serde::Serializer, { match self { - LayerFileName::Image(fname) => serializer.serialize_str(&format!("{}", fname)), - LayerFileName::Delta(fname) => serializer.serialize_str(&format!("{}", fname)), - #[cfg(test)] - LayerFileName::Test(t) => { - serializer.serialize_str(&format!("{LAYER_FILE_NAME_TEST_PREFIX}{t}")) - } + Self::Image(fname) => serializer.serialize_str(&fname.to_string()), + Self::Delta(fname) => serializer.serialize_str(&fname.to_string()), } } } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 22f1ce6e2d..e80da302b7 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -898,8 +898,6 @@ impl Timeline { &delta_name, &layer_metadata, ), - #[cfg(test)] - LayerFileName::Test(_) => unreachable!(), }); let gc_lock = self.layer_removal_cs.lock().await; @@ -1370,8 +1368,6 @@ impl Timeline { let remote_layer = Arc::new(remote_layer); updates.insert_historic(remote_layer); } - #[cfg(test)] - LayerFileName::Test(_) => unreachable!(), } }