From b9d2c7bdd555e5c99e1e8ab7f418be6647407a57 Mon Sep 17 00:00:00 2001 From: Vlad Lazar Date: Tue, 13 Aug 2024 12:45:54 +0100 Subject: [PATCH] pageserver: remove vectored get related configs (#8695) ## Problem Pageserver exposes some vectored get related configs which are not in use. ## Summary of changes Remove the following pageserver configs: get_impl, get_vectored_impl, and `validate_get_vectored`. They are not used in the pageserver since https://github.com/neondatabase/neon/pull/8601. Manual overrides have been removed from the aws repo in https://github.com/neondatabase/aws/pull/1664. --- pageserver/src/bin/pageserver.rs | 2 - pageserver/src/config.rs | 58 +------------------ pageserver/src/tenant/timeline.rs | 37 +----------- .../pagebench/test_large_slru_basebackup.py | 3 +- 4 files changed, 4 insertions(+), 96 deletions(-) diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index 932918410c..da0c11d9bf 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -124,8 +124,6 @@ fn main() -> anyhow::Result<()> { // after setting up logging, log the effective IO engine choice and read path implementations info!(?conf.virtual_file_io_engine, "starting with virtual_file IO engine"); info!(?conf.virtual_file_direct_io, "starting with virtual_file Direct IO settings"); - info!(?conf.get_impl, "starting with get page implementation"); - info!(?conf.get_vectored_impl, "starting with vectored get page implementation"); info!(?conf.compact_level0_phase1_value_access, "starting with setting for compact_level0_phase1_value_access"); let tenants_path = conf.tenants_path(); diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index f4c367bd4d..3ac5ac539f 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -29,12 +29,12 @@ use utils::{ logging::LogFormat, }; +use crate::l0_flush::L0FlushConfig; +use crate::tenant::config::TenantConfOpt; use crate::tenant::timeline::compaction::CompactL0Phase1ValueAccess; use crate::tenant::vectored_blob_io::MaxVectoredReadBytes; -use crate::tenant::{config::TenantConfOpt, timeline::GetImpl}; use crate::tenant::{TENANTS_SEGMENT_NAME, TIMELINES_SEGMENT_NAME}; use crate::{disk_usage_eviction_task::DiskUsageEvictionTaskConfig, virtual_file::io_engine}; -use crate::{l0_flush::L0FlushConfig, tenant::timeline::GetVectoredImpl}; use crate::{tenant::config::TenantConf, virtual_file}; use crate::{TENANT_HEATMAP_BASENAME, TENANT_LOCATION_CONFIG_NAME, TIMELINE_DELETE_MARK_SUFFIX}; @@ -133,14 +133,8 @@ pub mod defaults { #virtual_file_io_engine = '{DEFAULT_VIRTUAL_FILE_IO_ENGINE}' -#get_vectored_impl = '{DEFAULT_GET_VECTORED_IMPL}' - -#get_impl = '{DEFAULT_GET_IMPL}' - #max_vectored_read_bytes = '{DEFAULT_MAX_VECTORED_READ_BYTES}' -#validate_vectored_get = '{DEFAULT_VALIDATE_VECTORED_GET}' - [tenant_config] #checkpoint_distance = {DEFAULT_CHECKPOINT_DISTANCE} # in bytes #checkpoint_timeout = {DEFAULT_CHECKPOINT_TIMEOUT} @@ -278,14 +272,8 @@ pub struct PageServerConf { pub virtual_file_io_engine: virtual_file::IoEngineKind, - pub get_vectored_impl: GetVectoredImpl, - - pub get_impl: GetImpl, - pub max_vectored_read_bytes: MaxVectoredReadBytes, - pub validate_vectored_get: bool, - pub image_compression: ImageCompressionAlgorithm, /// How many bytes of ephemeral layer content will we allow per kilobyte of RAM. When this @@ -396,14 +384,8 @@ struct PageServerConfigBuilder { virtual_file_io_engine: BuilderValue, - get_vectored_impl: BuilderValue, - - get_impl: BuilderValue, - max_vectored_read_bytes: BuilderValue, - validate_vectored_get: BuilderValue, - image_compression: BuilderValue, ephemeral_bytes_per_memory_kb: BuilderValue, @@ -493,13 +475,10 @@ impl PageServerConfigBuilder { virtual_file_io_engine: Set(DEFAULT_VIRTUAL_FILE_IO_ENGINE.parse().unwrap()), - get_vectored_impl: Set(DEFAULT_GET_VECTORED_IMPL.parse().unwrap()), - get_impl: Set(DEFAULT_GET_IMPL.parse().unwrap()), max_vectored_read_bytes: Set(MaxVectoredReadBytes( NonZeroUsize::new(DEFAULT_MAX_VECTORED_READ_BYTES).unwrap(), )), image_compression: Set(DEFAULT_IMAGE_COMPRESSION), - validate_vectored_get: Set(DEFAULT_VALIDATE_VECTORED_GET), ephemeral_bytes_per_memory_kb: Set(DEFAULT_EPHEMERAL_BYTES_PER_MEMORY_KB), l0_flush: Set(L0FlushConfig::default()), compact_level0_phase1_value_access: Set(CompactL0Phase1ValueAccess::default()), @@ -659,22 +638,10 @@ impl PageServerConfigBuilder { self.virtual_file_io_engine = BuilderValue::Set(value); } - pub fn get_vectored_impl(&mut self, value: GetVectoredImpl) { - self.get_vectored_impl = BuilderValue::Set(value); - } - - pub fn get_impl(&mut self, value: GetImpl) { - self.get_impl = BuilderValue::Set(value); - } - pub fn get_max_vectored_read_bytes(&mut self, value: MaxVectoredReadBytes) { self.max_vectored_read_bytes = BuilderValue::Set(value); } - pub fn get_validate_vectored_get(&mut self, value: bool) { - self.validate_vectored_get = BuilderValue::Set(value); - } - pub fn get_image_compression(&mut self, value: ImageCompressionAlgorithm) { self.image_compression = BuilderValue::Set(value); } @@ -745,10 +712,7 @@ impl PageServerConfigBuilder { heatmap_upload_concurrency, secondary_download_concurrency, ingest_batch_size, - get_vectored_impl, - get_impl, max_vectored_read_bytes, - validate_vectored_get, image_compression, ephemeral_bytes_per_memory_kb, l0_flush, @@ -1002,21 +966,12 @@ impl PageServerConf { "virtual_file_io_engine" => { builder.virtual_file_io_engine(parse_toml_from_str("virtual_file_io_engine", item)?) } - "get_vectored_impl" => { - builder.get_vectored_impl(parse_toml_from_str("get_vectored_impl", item)?) - } - "get_impl" => { - builder.get_impl(parse_toml_from_str("get_impl", item)?) - } "max_vectored_read_bytes" => { let bytes = parse_toml_u64("max_vectored_read_bytes", item)? as usize; builder.get_max_vectored_read_bytes( MaxVectoredReadBytes( NonZeroUsize::new(bytes).expect("Max byte size of vectored read must be greater than 0"))) } - "validate_vectored_get" => { - builder.get_validate_vectored_get(parse_toml_bool("validate_vectored_get", item)?) - } "image_compression" => { builder.get_image_compression(parse_toml_from_str("image_compression", item)?) } @@ -1106,14 +1061,11 @@ impl PageServerConf { secondary_download_concurrency: defaults::DEFAULT_SECONDARY_DOWNLOAD_CONCURRENCY, ingest_batch_size: defaults::DEFAULT_INGEST_BATCH_SIZE, virtual_file_io_engine: DEFAULT_VIRTUAL_FILE_IO_ENGINE.parse().unwrap(), - get_vectored_impl: defaults::DEFAULT_GET_VECTORED_IMPL.parse().unwrap(), - get_impl: defaults::DEFAULT_GET_IMPL.parse().unwrap(), max_vectored_read_bytes: MaxVectoredReadBytes( NonZeroUsize::new(defaults::DEFAULT_MAX_VECTORED_READ_BYTES) .expect("Invalid default constant"), ), image_compression: defaults::DEFAULT_IMAGE_COMPRESSION, - validate_vectored_get: defaults::DEFAULT_VALIDATE_VECTORED_GET, ephemeral_bytes_per_memory_kb: defaults::DEFAULT_EPHEMERAL_BYTES_PER_MEMORY_KB, l0_flush: L0FlushConfig::default(), compact_level0_phase1_value_access: CompactL0Phase1ValueAccess::default(), @@ -1349,13 +1301,10 @@ background_task_maximum_delay = '334 s' secondary_download_concurrency: defaults::DEFAULT_SECONDARY_DOWNLOAD_CONCURRENCY, ingest_batch_size: defaults::DEFAULT_INGEST_BATCH_SIZE, virtual_file_io_engine: DEFAULT_VIRTUAL_FILE_IO_ENGINE.parse().unwrap(), - get_vectored_impl: defaults::DEFAULT_GET_VECTORED_IMPL.parse().unwrap(), - get_impl: defaults::DEFAULT_GET_IMPL.parse().unwrap(), max_vectored_read_bytes: MaxVectoredReadBytes( NonZeroUsize::new(defaults::DEFAULT_MAX_VECTORED_READ_BYTES) .expect("Invalid default constant") ), - validate_vectored_get: defaults::DEFAULT_VALIDATE_VECTORED_GET, image_compression: defaults::DEFAULT_IMAGE_COMPRESSION, ephemeral_bytes_per_memory_kb: defaults::DEFAULT_EPHEMERAL_BYTES_PER_MEMORY_KB, l0_flush: L0FlushConfig::default(), @@ -1425,13 +1374,10 @@ background_task_maximum_delay = '334 s' secondary_download_concurrency: defaults::DEFAULT_SECONDARY_DOWNLOAD_CONCURRENCY, ingest_batch_size: 100, virtual_file_io_engine: DEFAULT_VIRTUAL_FILE_IO_ENGINE.parse().unwrap(), - get_vectored_impl: defaults::DEFAULT_GET_VECTORED_IMPL.parse().unwrap(), - get_impl: defaults::DEFAULT_GET_IMPL.parse().unwrap(), max_vectored_read_bytes: MaxVectoredReadBytes( NonZeroUsize::new(defaults::DEFAULT_MAX_VECTORED_READ_BYTES) .expect("Invalid default constant") ), - validate_vectored_get: defaults::DEFAULT_VALIDATE_VECTORED_GET, image_compression: defaults::DEFAULT_IMAGE_COMPRESSION, ephemeral_bytes_per_memory_kb: defaults::DEFAULT_EPHEMERAL_BYTES_PER_MEMORY_KB, l0_flush: L0FlushConfig::default(), diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 9bce9c1fac..abe3f56e45 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -802,40 +802,6 @@ impl From for PageReconstructError { } } -#[derive( - Eq, - PartialEq, - Debug, - Copy, - Clone, - strum_macros::EnumString, - strum_macros::Display, - serde_with::DeserializeFromStr, - serde_with::SerializeDisplay, -)] -#[strum(serialize_all = "kebab-case")] -pub enum GetVectoredImpl { - Sequential, - Vectored, -} - -#[derive( - Eq, - PartialEq, - Debug, - Copy, - Clone, - strum_macros::EnumString, - strum_macros::Display, - serde_with::DeserializeFromStr, - serde_with::SerializeDisplay, -)] -#[strum(serialize_all = "kebab-case")] -pub enum GetImpl { - Legacy, - Vectored, -} - pub(crate) enum WaitLsnWaiter<'a> { Timeline(&'a Timeline), Tenant, @@ -995,11 +961,10 @@ impl Timeline { } trace!( - "get vectored request for {:?}@{} from task kind {:?} will use {} implementation", + "get vectored request for {:?}@{} from task kind {:?}", keyspace, lsn, ctx.task_kind(), - self.conf.get_vectored_impl ); let start = crate::metrics::GET_VECTORED_LATENCY diff --git a/test_runner/performance/pageserver/pagebench/test_large_slru_basebackup.py b/test_runner/performance/pageserver/pagebench/test_large_slru_basebackup.py index 3258d4dcfa..8b934057e4 100644 --- a/test_runner/performance/pageserver/pagebench/test_large_slru_basebackup.py +++ b/test_runner/performance/pageserver/pagebench/test_large_slru_basebackup.py @@ -44,8 +44,7 @@ def test_basebackup_with_high_slru_count( page_cache_size = 16384 max_file_descriptors = 500000 neon_env_builder.pageserver_config_override = ( - f"page_cache_size={page_cache_size}; max_file_descriptors={max_file_descriptors}; " - f"get_vectored_impl='vectored'; validate_vectored_get=false" + f"page_cache_size={page_cache_size}; max_file_descriptors={max_file_descriptors}" ) params.update( {