From 515dd4ee666daf1d4e932de2b6bb126e364528dc Mon Sep 17 00:00:00 2001 From: Alex Chi Z Date: Thu, 29 Aug 2024 14:32:09 -0400 Subject: [PATCH] rm more dead code Signed-off-by: Alex Chi Z --- pageserver/src/http/routes.rs | 1 - pageserver/src/pgdatadir_mapping.rs | 43 +----------- pageserver/src/tenant.rs | 16 +---- .../src/tenant/remote_timeline_client.rs | 14 +--- .../tenant/remote_timeline_client/index.rs | 6 +- pageserver/src/tenant/timeline.rs | 16 ++--- pageserver/src/tenant/timeline/delete.rs | 2 - pageserver/src/walredo/apply_neon.rs | 67 +------------------ 8 files changed, 12 insertions(+), 153 deletions(-) diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 2dc6a77a60..1d3b222b1c 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -17,7 +17,6 @@ use hyper::header; use hyper::StatusCode; use hyper::{Body, Request, Response, Uri}; use metrics::launch_timestamp::LaunchTimestamp; -use pageserver_api::models::AuxFilePolicy; use pageserver_api::models::DownloadRemoteLayersTaskSpawnRequest; use pageserver_api::models::IngestAuxFilesRequest; use pageserver_api::models::ListAuxFilesRequest; diff --git a/pageserver/src/pgdatadir_mapping.rs b/pageserver/src/pgdatadir_mapping.rs index f0723e019b..2b40daa9e4 100644 --- a/pageserver/src/pgdatadir_mapping.rs +++ b/pageserver/src/pgdatadir_mapping.rs @@ -19,10 +19,9 @@ use pageserver_api::key::{ dbdir_key_range, rel_block_to_key, rel_dir_to_key, rel_key_range, rel_size_to_key, relmap_file_key, repl_origin_key, repl_origin_key_range, slru_block_to_key, slru_dir_to_key, slru_segment_key_range, slru_segment_size_to_key, twophase_file_key, twophase_key_range, - CompactKey, AUX_FILES_KEY, CHECKPOINT_KEY, CONTROLFILE_KEY, DBDIR_KEY, TWOPHASEDIR_KEY, + CompactKey, CHECKPOINT_KEY, CONTROLFILE_KEY, DBDIR_KEY, TWOPHASEDIR_KEY, }; use pageserver_api::keyspace::SparseKeySpace; -use pageserver_api::models::AuxFilePolicy; use pageserver_api::reltag::{BlockNumber, RelTag, SlruKind}; use postgres_ffi::relfile_utils::{FSM_FORKNUM, VISIBILITYMAP_FORKNUM}; use postgres_ffi::BLCKSZ; @@ -1023,9 +1022,6 @@ impl<'a> DatadirModification<'a> { self.pending_directory_entries.push((DirectoryKind::Db, 0)); self.put(DBDIR_KEY, Value::Image(buf.into())); - // Create AuxFilesDirectory - self.init_aux_dir()?; - let buf = TwoPhaseDirectory::ser(&TwoPhaseDirectory { xids: HashSet::new(), })?; @@ -1138,9 +1134,6 @@ impl<'a> DatadirModification<'a> { // 'true', now write the updated 'dbdirs' map back. let buf = DbDirectory::ser(&dbdir)?; self.put(DBDIR_KEY, Value::Image(buf.into())); - - // Create AuxFilesDirectory as well - self.init_aux_dir()?; } if r.is_none() { // Create RelDirectory @@ -1501,19 +1494,6 @@ impl<'a> DatadirModification<'a> { Ok(()) } - pub fn init_aux_dir(&mut self) -> anyhow::Result<()> { - if let AuxFilePolicy::V2 = self.tline.get_switch_aux_file_policy() { - return Ok(()); - } - let buf = AuxFilesDirectory::ser(&AuxFilesDirectory { - files: HashMap::new(), - })?; - self.pending_directory_entries - .push((DirectoryKind::AuxFiles, 0)); - self.put(AUX_FILES_KEY, Value::Image(Bytes::from(buf))); - Ok(()) - } - pub async fn put_file( &mut self, path: &str, @@ -1730,12 +1710,6 @@ impl<'a> DatadirModification<'a> { self.tline.get(key, lsn, ctx).await } - /// Only used during unit tests, force putting a key into the modification. - #[cfg(test)] - pub(crate) fn put_for_test(&mut self, key: Key, val: Value) { - self.put(key, val); - } - fn put(&mut self, key: Key, val: Value) { let values = self.pending_updates.entry(key).or_default(); // Replace the previous value if it exists at the same lsn @@ -1813,21 +1787,6 @@ struct RelDirectory { rels: HashSet<(Oid, u8)>, } -#[derive(Debug, Serialize, Deserialize, Default, PartialEq)] -pub(crate) struct AuxFilesDirectory { - pub(crate) files: HashMap, -} - -impl AuxFilesDirectory { - pub(crate) fn upsert(&mut self, key: String, value: Option) { - if let Some(value) = value { - self.files.insert(key, value); - } else { - self.files.remove(&key); - } - } -} - #[derive(Debug, Serialize, Deserialize)] struct RelSizeEntry { nblocks: u32, diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 90eaecaabe..3bd335329e 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -20,7 +20,6 @@ use futures::stream::FuturesUnordered; use futures::FutureExt; use futures::StreamExt; use pageserver_api::models; -use pageserver_api::models::AuxFilePolicy; use pageserver_api::models::TimelineArchivalState; use pageserver_api::models::TimelineState; use pageserver_api::models::TopTenantShardItem; @@ -694,10 +693,6 @@ impl Tenant { ancestor.clone(), resources, CreateTimelineCause::Load, - // This could be derived from ancestor branch + index part. Though the only caller of `timeline_init_and_sync` is `load_remote_timeline`, - // there will potentially be other caller of this function in the future, and we don't know whether `index_part` or `ancestor` takes precedence. - // Therefore, we pass this field explicitly for now, and remove it once we fully migrate to aux file v2. - last_aux_file_policy, )?; let disk_consistent_lsn = timeline.get_disk_consistent_lsn(); anyhow::ensure!( @@ -1282,15 +1277,12 @@ impl Tenant { None }; - let last_aux_file_policy = index_part.last_aux_file_policy(); - self.timeline_init_and_sync( timeline_id, resources, Some(index_part), remote_metadata, ancestor, - last_aux_file_policy, ctx, ) .await @@ -1507,7 +1499,6 @@ impl Tenant { create_guard, initdb_lsn, None, - None, ) .await } @@ -2669,7 +2660,6 @@ impl Tenant { ancestor: Option>, resources: TimelineResources, cause: CreateTimelineCause, - last_aux_file_policy: Option, ) -> anyhow::Result> { let state = match cause { CreateTimelineCause::Load => { @@ -2698,7 +2688,6 @@ impl Tenant { resources, pg_version, state, - last_aux_file_policy, self.cancel.child_token(), ); @@ -3600,7 +3589,6 @@ impl Tenant { ancestor, resources, CreateTimelineCause::Load, - last_aux_file_policy, ) .context("Failed to create timeline data structure")?; @@ -4189,7 +4177,6 @@ mod tests { use super::*; use crate::keyspace::KeySpaceAccum; - use crate::pgdatadir_mapping::AuxFilesDirectory; use crate::repository::{Key, Value}; use crate::tenant::harness::*; use crate::tenant::timeline::CompactFlags; @@ -4198,7 +4185,7 @@ mod tests { use bytes::{Bytes, BytesMut}; use hex_literal::hex; use itertools::Itertools; - use pageserver_api::key::{AUX_FILES_KEY, AUX_KEY_PREFIX, NON_INHERITED_RANGE}; + use pageserver_api::key::{AUX_KEY_PREFIX, NON_INHERITED_RANGE}; use pageserver_api::keyspace::KeySpace; use pageserver_api::models::{CompactionAlgorithm, CompactionAlgorithmSettings}; use rand::{thread_rng, Rng}; @@ -4207,7 +4194,6 @@ mod tests { use tests::timeline::{GetVectoredError, ShutdownMode}; use timeline::compaction::{KeyHistoryRetention, KeyLogAtLsn}; use timeline::{DeltaLayerTestDesc, GcInfo}; - use utils::bin_ser::BeSer; use utils::id::TenantId; static TEST_KEY: Lazy = diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index 71b766e4c7..1e6575e409 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -187,7 +187,7 @@ use camino::Utf8Path; use chrono::{NaiveDateTime, Utc}; pub(crate) use download::download_initdb_tar_zst; -use pageserver_api::models::{AuxFilePolicy, TimelineArchivalState}; +use pageserver_api::models::TimelineArchivalState; use pageserver_api::shard::{ShardIndex, TenantShardId}; use scopeguard::ScopeGuard; use tokio_util::sync::CancellationToken; @@ -628,18 +628,6 @@ impl RemoteTimelineClient { Ok(()) } - /// Launch an index-file upload operation in the background, with only the `aux_file_policy` flag updated. - pub(crate) fn schedule_index_upload_for_aux_file_policy_update( - self: &Arc, - last_aux_file_policy: Option, - ) -> anyhow::Result<()> { - let mut guard = self.upload_queue.lock().unwrap(); - let upload_queue = guard.initialized_mut()?; - upload_queue.dirty.last_aux_file_policy = last_aux_file_policy; - self.schedule_index_upload(upload_queue)?; - Ok(()) - } - /// Launch an index-file upload operation in the background, with only the `archived_at` field updated. /// /// Returns whether it is required to wait for the queue to be empty to ensure that the change is uploaded, diff --git a/pageserver/src/tenant/remote_timeline_client/index.rs b/pageserver/src/tenant/remote_timeline_client/index.rs index 32a1b8188f..be0cb0057b 100644 --- a/pageserver/src/tenant/remote_timeline_client/index.rs +++ b/pageserver/src/tenant/remote_timeline_client/index.rs @@ -70,7 +70,7 @@ pub struct IndexPart { /// /// None means no aux files have been written to the storage before the point /// when this flag is introduced. - /// + /// /// This field is deprecated as part of the aux v1 retirement. #[serde(skip_serializing_if = "Option::is_none", default)] pub(crate) last_aux_file_policy: Option, @@ -134,10 +134,6 @@ impl IndexPart { pub(crate) fn example() -> Self { Self::empty(TimelineMetadata::example()) } - - pub(crate) fn last_aux_file_policy(&self) -> Option { - self.last_aux_file_policy - } } /// Metadata gathered for each of the layer files. diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index c161cda61d..3ce83501ff 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -27,7 +27,7 @@ use pageserver_api::{ }, keyspace::{KeySpaceAccum, KeySpaceRandomAccum, SparseKeyPartitioning}, models::{ - AtomicAuxFilePolicy, AuxFilePolicy, CompactionAlgorithm, CompactionAlgorithmSettings, + AuxFilePolicy, CompactionAlgorithm, CompactionAlgorithmSettings, DownloadRemoteLayersTaskInfo, DownloadRemoteLayersTaskSpawnRequest, EvictionPolicy, InMemoryLayerInfo, LayerMapInfo, LsnLease, TimelineState, }, @@ -96,12 +96,12 @@ use crate::{ use crate::{ metrics::ScanLatencyOngoingRecording, tenant::timeline::logical_size::CurrentLogicalSize, }; -use crate::{pgdatadir_mapping::LsnForTimestamp, tenant::tasks::BackgroundLoopKind}; -use crate::{pgdatadir_mapping::MAX_AUX_FILE_V2_DELTAS, tenant::storage_layer::PersistentLayerKey}; use crate::{ - pgdatadir_mapping::{AuxFilesDirectory, DirectoryKind}, + pgdatadir_mapping::DirectoryKind, virtual_file::{MaybeFatalIo, VirtualFile}, }; +use crate::{pgdatadir_mapping::LsnForTimestamp, tenant::tasks::BackgroundLoopKind}; +use crate::{pgdatadir_mapping::MAX_AUX_FILE_V2_DELTAS, tenant::storage_layer::PersistentLayerKey}; use crate::config::PageServerConf; use crate::keyspace::{KeyPartitioning, KeySpace}; @@ -1938,7 +1938,7 @@ impl Timeline { } // TODO(yuchen): remove unused flag after implementing https://github.com/neondatabase/neon/issues/8072 - #[allow(unused)] + #[allow(dead_code)] pub(crate) fn get_lsn_lease_length_for_ts(&self) -> Duration { let tenant_conf = self.tenant_conf.load(); tenant_conf @@ -1947,6 +1947,7 @@ impl Timeline { .unwrap_or(self.conf.default_tenant_conf.lsn_lease_length_for_ts) } + #[allow(dead_code)] pub(crate) fn get_switch_aux_file_policy(&self) -> AuxFilePolicy { let tenant_conf = self.tenant_conf.load(); tenant_conf @@ -2087,7 +2088,6 @@ impl Timeline { resources: TimelineResources, pg_version: u32, state: TimelineState, - aux_file_policy: Option, cancel: CancellationToken, ) -> Arc { let disk_consistent_lsn = metadata.disk_consistent_lsn(); @@ -2224,10 +2224,6 @@ impl Timeline { handles: Default::default(), }; - if aux_file_policy == Some(AuxFilePolicy::V1) { - warn!("this timeline is using deprecated aux file policy V1"); - } - result.repartition_threshold = result.get_checkpoint_distance() / REPARTITION_FREQ_IN_CHECKPOINT_DISTANCE; diff --git a/pageserver/src/tenant/timeline/delete.rs b/pageserver/src/tenant/timeline/delete.rs index dc4118bb4a..b792943f57 100644 --- a/pageserver/src/tenant/timeline/delete.rs +++ b/pageserver/src/tenant/timeline/delete.rs @@ -286,8 +286,6 @@ impl DeleteTimelineFlow { // Important. We dont pass ancestor above because it can be missing. // Thus we need to skip the validation here. CreateTimelineCause::Delete, - // Aux file policy is not needed for deletion, assuming deletion does not read aux keyspace - None, ) .context("create_timeline_struct")?; diff --git a/pageserver/src/walredo/apply_neon.rs b/pageserver/src/walredo/apply_neon.rs index facf01004c..fb344ed430 100644 --- a/pageserver/src/walredo/apply_neon.rs +++ b/pageserver/src/walredo/apply_neon.rs @@ -1,4 +1,3 @@ -use crate::pgdatadir_mapping::AuxFilesDirectory; use crate::walrecord::NeonWalRecord; use anyhow::Context; use byteorder::{ByteOrder, LittleEndian}; @@ -13,7 +12,6 @@ use postgres_ffi::v14::nonrelfile_utils::{ }; use postgres_ffi::BLCKSZ; use tracing::*; -use utils::bin_ser::BeSer; use utils::lsn::Lsn; /// Can this request be served by neon redo functions @@ -236,13 +234,8 @@ pub(crate) fn apply_in_neon( LittleEndian::write_u32(&mut page[memberoff..memberoff + 4], member.xid); } } - NeonWalRecord::AuxFile { file_path, content } => { - let mut dir = AuxFilesDirectory::des(page)?; - dir.upsert(file_path.clone(), content.clone()); - - page.clear(); - let mut writer = page.writer(); - dir.ser_into(&mut writer)?; + NeonWalRecord::AuxFile { .. } => { + // No-op: this record will never be created in aux v2. } #[cfg(test)] NeonWalRecord::Test { @@ -261,59 +254,3 @@ pub(crate) fn apply_in_neon( } Ok(()) } - -#[cfg(test)] -mod test { - use bytes::Bytes; - use pageserver_api::key::AUX_FILES_KEY; - - use super::*; - use std::collections::HashMap; - - /// Test [`apply_in_neon`]'s handling of NeonWalRecord::AuxFile - #[test] - fn apply_aux_file_deltas() -> anyhow::Result<()> { - let base_dir = AuxFilesDirectory { - files: HashMap::from([ - ("two".to_string(), Bytes::from_static(b"content0")), - ("three".to_string(), Bytes::from_static(b"contentX")), - ]), - }; - let base_image = AuxFilesDirectory::ser(&base_dir)?; - - let deltas = vec![ - // Insert - NeonWalRecord::AuxFile { - file_path: "one".to_string(), - content: Some(Bytes::from_static(b"content1")), - }, - // Update - NeonWalRecord::AuxFile { - file_path: "two".to_string(), - content: Some(Bytes::from_static(b"content99")), - }, - // Delete - NeonWalRecord::AuxFile { - file_path: "three".to_string(), - content: None, - }, - ]; - - let file_path = AUX_FILES_KEY; - let mut page = BytesMut::from_iter(base_image); - - for record in deltas { - apply_in_neon(&record, Lsn(8), file_path, &mut page)?; - } - - let reconstructed = AuxFilesDirectory::des(&page)?; - let expect = HashMap::from([ - ("one".to_string(), Bytes::from_static(b"content1")), - ("two".to_string(), Bytes::from_static(b"content99")), - ]); - - assert_eq!(reconstructed.files, expect); - - Ok(()) - } -}