Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compaction_level0_phase1: bypass PS PageCache for data blocks #8543

Merged
merged 14 commits into from
Jul 31, 2024
Prev Previous commit
Next Next commit
renamings & docs
problame committed Jul 30, 2024
commit fdf4f1bd7be8852fa9cb120077961f0ea445d781
2 changes: 1 addition & 1 deletion pageserver/src/bin/pageserver.rs
Original file line number Diff line number Diff line change
@@ -129,7 +129,7 @@ fn main() -> anyhow::Result<()> {
info!(?conf.virtual_file_io_engine, "starting with virtual_file IO engine");
info!(?conf.get_impl, "starting with get page implementation");
info!(?conf.get_vectored_impl, "starting with vectored get page implementation");
info!(?conf.compact_level0_bypass_page_cache, "starting with setting for compact_level0_bypass_page_cache");
info!(?conf.compact_level0_phase1_value_access, "starting with setting for compact_level0_phase1_value_access");

let tenants_path = conf.tenants_path();
if !tenants_path.exists() {
27 changes: 14 additions & 13 deletions pageserver/src/config.rs
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ use utils::{
logging::LogFormat,
};

use crate::tenant::timeline::compaction::CompactL0BypassPageCache;
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};
@@ -297,8 +297,9 @@ pub struct PageServerConf {

pub l0_flush: L0FlushConfig,

// TODO(https://github.com/neondatabase/neon/issues/8184): remove this flag at end of feature
pub compact_level0_bypass_page_cache: CompactL0BypassPageCache,
/// This flag is temporary and will be removed after gradual rollout.
/// See <https://github.com/neondatabase/neon/issues/8184>.
pub compact_level0_phase1_value_access: CompactL0Phase1ValueAccess,
}

/// We do not want to store this in a PageServerConf because the latter may be logged
@@ -406,7 +407,7 @@ struct PageServerConfigBuilder {

l0_flush: BuilderValue<L0FlushConfig>,

compact_level0_bypass_page_cache: BuilderValue<CompactL0BypassPageCache>,
compact_level0_phase1_value_access: BuilderValue<CompactL0Phase1ValueAccess>,
}

impl PageServerConfigBuilder {
@@ -496,7 +497,7 @@ impl PageServerConfigBuilder {
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_bypass_page_cache: Set(CompactL0BypassPageCache::default()),
compact_level0_phase1_value_access: Set(CompactL0Phase1ValueAccess::default()),
}
}
}
@@ -680,8 +681,8 @@ impl PageServerConfigBuilder {
self.l0_flush = BuilderValue::Set(value);
}

pub fn compact_level0_bypass_page_cache(&mut self, value: CompactL0BypassPageCache) {
self.compact_level0_bypass_page_cache = BuilderValue::Set(value);
pub fn compact_level0_phase1_value_access(&mut self, value: CompactL0Phase1ValueAccess) {
self.compact_level0_phase1_value_access = BuilderValue::Set(value);
}

pub fn build(self, id: NodeId) -> anyhow::Result<PageServerConf> {
@@ -741,7 +742,7 @@ impl PageServerConfigBuilder {
image_compression,
ephemeral_bytes_per_memory_kb,
l0_flush,
compact_level0_bypass_page_cache,
compact_level0_phase1_value_access,
}
CUSTOM LOGIC
{
@@ -1014,8 +1015,8 @@ impl PageServerConf {
"l0_flush" => {
builder.l0_flush(utils::toml_edit_ext::deserialize_item(item).context("l0_flush")?)
}
"compact_level0_bypass_page_cache" => {
builder.compact_level0_bypass_page_cache(utils::toml_edit_ext::deserialize_item(item).context("compact_level0_bypass_page_cache")?)
"compact_level0_phase1_value_access" => {
builder.compact_level0_phase1_value_access(utils::toml_edit_ext::deserialize_item(item).context("compact_level0_phase1_value_access")?)
}
_ => bail!("unrecognized pageserver option '{key}'"),
}
@@ -1101,7 +1102,7 @@ impl PageServerConf {
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_bypass_page_cache: CompactL0BypassPageCache::default(),
compact_level0_phase1_value_access: CompactL0Phase1ValueAccess::default(),
}
}
}
@@ -1343,7 +1344,7 @@ background_task_maximum_delay = '334 s'
image_compression: defaults::DEFAULT_IMAGE_COMPRESSION,
ephemeral_bytes_per_memory_kb: defaults::DEFAULT_EPHEMERAL_BYTES_PER_MEMORY_KB,
l0_flush: L0FlushConfig::default(),
compact_level0_bypass_page_cache: CompactL0BypassPageCache::default(),
compact_level0_phase1_value_access: CompactL0Phase1ValueAccess::default(),
},
"Correct defaults should be used when no config values are provided"
);
@@ -1418,7 +1419,7 @@ background_task_maximum_delay = '334 s'
image_compression: defaults::DEFAULT_IMAGE_COMPRESSION,
ephemeral_bytes_per_memory_kb: defaults::DEFAULT_EPHEMERAL_BYTES_PER_MEMORY_KB,
l0_flush: L0FlushConfig::default(),
compact_level0_bypass_page_cache: CompactL0BypassPageCache::default(),
compact_level0_phase1_value_access: CompactL0Phase1ValueAccess::default(),
},
"Should be able to parse all basic config values correctly"
);
39 changes: 25 additions & 14 deletions pageserver/src/tenant/timeline/compaction.rs
Original file line number Diff line number Diff line change
@@ -705,7 +705,7 @@ impl Timeline {
// option and validation code once we've reached confidence.
enum AllValuesIter<'a> {
#[allow(dead_code)]
problame marked this conversation as resolved.
Show resolved Hide resolved
PageCachedBlobIoWithLoadedIndex { all_keys_iter: VecIter<'a> },
PageCachedBlobIo { all_keys_iter: VecIter<'a> },
#[allow(dead_code)]
StreamingKmergeBypassingPageCache { merge_iter: MergeIterator<'a> },
ValidatingStreamingKmergeBypassingPageCache {
@@ -737,7 +737,7 @@ impl Timeline {
ctx: &RequestContext,
) -> anyhow::Result<Option<(Key, Lsn, Value)>> {
match self {
AllValuesIter::PageCachedBlobIoWithLoadedIndex { all_keys_iter: iter } => {
AllValuesIter::PageCachedBlobIo { all_keys_iter: iter } => {
Self::next_all_keys_iter(iter, ctx).await
}
AllValuesIter::StreamingKmergeBypassingPageCache { merge_iter } => merge_iter.next().await,
@@ -807,13 +807,11 @@ impl Timeline {
}
}
}
let mut all_values_iter = match &self.conf.compact_level0_bypass_page_cache {
CompactL0BypassPageCache::PageCachedBlobIoWithLoadedIndex => {
AllValuesIter::PageCachedBlobIoWithLoadedIndex {
all_keys_iter: all_keys.iter(),
}
}
CompactL0BypassPageCache::StreamingKmergeBypassingPageCache { validate } => {
let mut all_values_iter = match &self.conf.compact_level0_phase1_value_access {
CompactL0Phase1ValueAccess::PageCachedBlobIo => AllValuesIter::PageCachedBlobIo {
all_keys_iter: all_keys.iter(),
},
CompactL0Phase1ValueAccess::StreamingKmerge { validate } => {
let merge_iter = {
let mut deltas = Vec::with_capacity(deltas_to_compact.len());
for l in deltas_to_compact.iter() {
@@ -1206,22 +1204,35 @@ impl TryFrom<CompactLevel0Phase1StatsBuilder> for CompactLevel0Phase1Stats {

#[derive(Debug, PartialEq, Eq, Clone, serde::Deserialize, serde::Serialize)]
#[serde(tag = "mode", rename_all = "kebab-case", deny_unknown_fields)]
pub enum CompactL0BypassPageCache {
PageCachedBlobIoWithLoadedIndex,
StreamingKmergeBypassingPageCache {
pub enum CompactL0Phase1ValueAccess {
/// The old way.
PageCachedBlobIo,
/// The new way.
StreamingKmerge {
/// If set, we run both the old way and the new way, validate that
/// they are identical (=> [`CompactL0BypassPageCacheValidation`]),
/// and if the validation fails,
/// - in tests: fail them with a panic or
/// - in prod, log a rate-limited warning and use the old way's results.
///
/// If not set, we only run the new way and trust its results.
validate: Option<CompactL0BypassPageCacheValidation>,
},
}

/// See [`CompactL0Phase1ValueAccess::StreamingKmerge`].
#[derive(Debug, PartialEq, Eq, Clone, serde::Deserialize, serde::Serialize)]
#[serde(rename_all = "kebab-case")]
pub enum CompactL0BypassPageCacheValidation {
/// Validate that the series of (key, lsn) pairs are the same.
KeyLsn,
/// Validate that the entire output of old and new way is identical.
KeyLsnValue,
}

impl Default for CompactL0BypassPageCache {
impl Default for CompactL0Phase1ValueAccess {
fn default() -> Self {
CompactL0BypassPageCache::StreamingKmergeBypassingPageCache {
CompactL0Phase1ValueAccess::StreamingKmerge {
// TODO(https://github.com/neondatabase/neon/issues/8184): change to None once confident
validate: Some(CompactL0BypassPageCacheValidation::KeyLsnValue),
}