From 02c6abadf01fb9dd3e795b8b1e1b96e866366bd8 Mon Sep 17 00:00:00 2001 From: Vlad Lazar Date: Fri, 12 Jan 2024 17:11:19 +0000 Subject: [PATCH] pageserver: remove depenency of pagebench on pageserver (#6334) To achieve this I had to lift the BlockNumber and key_to_rel_block definitions to pageserver_api (similar to a change in #5980). Closes #6299 --- Cargo.lock | 1 - libs/pageserver_api/src/key.rs | 18 +++++++++++++++++ libs/pageserver_api/src/reltag.rs | 3 +++ pageserver/pagebench/Cargo.toml | 1 - .../pagebench/src/cmd/getpage_latest_lsn.rs | 8 +++----- pageserver/src/pgdatadir_mapping.rs | 20 +------------------ pageserver/src/tenant/timeline.rs | 5 ++--- pageserver/src/walingest.rs | 2 +- pageserver/src/walredo.rs | 4 +++- 9 files changed, 31 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d3b0d947f..c5c2523e29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3217,7 +3217,6 @@ dependencies = [ "hdrhistogram", "humantime", "humantime-serde", - "pageserver", "pageserver_api", "pageserver_client", "rand 0.8.5", diff --git a/libs/pageserver_api/src/key.rs b/libs/pageserver_api/src/key.rs index e00d15e494..6a3679292e 100644 --- a/libs/pageserver_api/src/key.rs +++ b/libs/pageserver_api/src/key.rs @@ -3,6 +3,8 @@ use byteorder::{ByteOrder, BE}; use serde::{Deserialize, Serialize}; use std::fmt; +use crate::reltag::{BlockNumber, RelTag}; + /// Key used in the Repository kv-store. /// /// The Repository treats this as an opaque struct, but see the code in pgdatadir_mapping.rs @@ -146,6 +148,22 @@ pub fn is_rel_block_key(key: &Key) -> bool { key.field1 == 0x00 && key.field4 != 0 && key.field6 != 0xffffffff } +/// Guaranteed to return `Ok()` if [[is_rel_block_key]] returns `true` for `key`. +pub fn key_to_rel_block(key: Key) -> anyhow::Result<(RelTag, BlockNumber)> { + Ok(match key.field1 { + 0x00 => ( + RelTag { + spcnode: key.field2, + dbnode: key.field3, + relnode: key.field4, + forknum: key.field5, + }, + key.field6, + ), + _ => anyhow::bail!("unexpected value kind 0x{:02x}", key.field1), + }) +} + impl std::str::FromStr for Key { type Err = anyhow::Error; diff --git a/libs/pageserver_api/src/reltag.rs b/libs/pageserver_api/src/reltag.rs index 33402ca8ba..e3a7da2ad9 100644 --- a/libs/pageserver_api/src/reltag.rs +++ b/libs/pageserver_api/src/reltag.rs @@ -32,6 +32,9 @@ pub struct RelTag { pub relnode: Oid, } +/// Block number within a relation or SLRU. This matches PostgreSQL's BlockNumber type. +pub type BlockNumber = u32; + impl PartialOrd for RelTag { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) diff --git a/pageserver/pagebench/Cargo.toml b/pageserver/pagebench/Cargo.toml index 70fafee629..245d293e4f 100644 --- a/pageserver/pagebench/Cargo.toml +++ b/pageserver/pagebench/Cargo.toml @@ -21,7 +21,6 @@ tracing.workspace = true tokio.workspace = true tokio-util.workspace = true -pageserver = { path = ".." } pageserver_client.workspace = true pageserver_api.workspace = true utils = { path = "../../libs/utils/" } diff --git a/pageserver/pagebench/src/cmd/getpage_latest_lsn.rs b/pageserver/pagebench/src/cmd/getpage_latest_lsn.rs index 38d5bf4e61..46b8bd10d7 100644 --- a/pageserver/pagebench/src/cmd/getpage_latest_lsn.rs +++ b/pageserver/pagebench/src/cmd/getpage_latest_lsn.rs @@ -1,9 +1,7 @@ use anyhow::Context; use camino::Utf8PathBuf; use futures::future::join_all; -use pageserver::pgdatadir_mapping::key_to_rel_block; -use pageserver::repository; -use pageserver_api::key::is_rel_block_key; +use pageserver_api::key::{is_rel_block_key, key_to_rel_block, Key}; use pageserver_api::keyspace::KeySpaceAccum; use pageserver_api::models::PagestreamGetPageRequest; @@ -269,7 +267,7 @@ async fn main_impl( let mut rng = rand::thread_rng(); let r = &all_ranges[weights.sample(&mut rng)]; let key: i128 = rng.gen_range(r.start..r.end); - let key = repository::Key::from_i128(key); + let key = Key::from_i128(key); let (rel_tag, block_no) = key_to_rel_block(key).expect("we filter non-rel-block keys out above"); ( @@ -319,7 +317,7 @@ async fn main_impl( let mut rng = rand::thread_rng(); let r = &ranges[weights.sample(&mut rng)]; let key: i128 = rng.gen_range(r.start..r.end); - let key = repository::Key::from_i128(key); + let key = Key::from_i128(key); assert!(is_rel_block_key(&key)); let (rel_tag, block_no) = key_to_rel_block(key) .expect("we filter non-rel-block keys out above"); diff --git a/pageserver/src/pgdatadir_mapping.rs b/pageserver/src/pgdatadir_mapping.rs index f11a72f2ab..d9cc85319e 100644 --- a/pageserver/src/pgdatadir_mapping.rs +++ b/pageserver/src/pgdatadir_mapping.rs @@ -14,7 +14,7 @@ use crate::walrecord::NeonWalRecord; use anyhow::{ensure, Context}; use bytes::{Buf, Bytes}; use pageserver_api::key::is_rel_block_key; -use pageserver_api::reltag::{RelTag, SlruKind}; +use pageserver_api::reltag::{BlockNumber, RelTag, SlruKind}; use postgres_ffi::relfile_utils::{FSM_FORKNUM, VISIBILITYMAP_FORKNUM}; use postgres_ffi::BLCKSZ; use postgres_ffi::{Oid, TimestampTz, TransactionId}; @@ -27,9 +27,6 @@ use tracing::{debug, trace, warn}; use utils::bin_ser::DeserializeError; use utils::{bin_ser::BeSer, lsn::Lsn}; -/// Block number within a relation or SLRU. This matches PostgreSQL's BlockNumber type. -pub type BlockNumber = u32; - #[derive(Debug)] pub enum LsnForTimestamp { /// Found commits both before and after the given timestamp @@ -1863,21 +1860,6 @@ pub fn is_inherited_key(key: Key) -> bool { key != AUX_FILES_KEY } -/// Guaranteed to return `Ok()` if [[is_rel_block_key]] returns `true` for `key`. -pub fn key_to_rel_block(key: Key) -> anyhow::Result<(RelTag, BlockNumber)> { - Ok(match key.field1 { - 0x00 => ( - RelTag { - spcnode: key.field2, - dbnode: key.field3, - relnode: key.field4, - forknum: key.field5, - }, - key.field6, - ), - _ => anyhow::bail!("unexpected value kind 0x{:02x}", key.field1), - }) -} pub fn is_rel_fsm_block_key(key: Key) -> bool { key.field1 == 0x00 && key.field4 != 0 && key.field5 == FSM_FORKNUM && key.field6 != 0xffffffff } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index ea1ab1a828..8b96c30522 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -65,11 +65,10 @@ use crate::keyspace::{KeyPartitioning, KeySpace, KeySpaceRandomAccum}; use crate::metrics::{ TimelineMetrics, MATERIALIZED_PAGE_CACHE_HIT, MATERIALIZED_PAGE_CACHE_HIT_DIRECT, }; -use crate::pgdatadir_mapping::LsnForTimestamp; use crate::pgdatadir_mapping::{is_inherited_key, is_rel_fsm_block_key, is_rel_vm_block_key}; -use crate::pgdatadir_mapping::{BlockNumber, CalculateLogicalSizeError}; +use crate::pgdatadir_mapping::{CalculateLogicalSizeError, LsnForTimestamp}; use crate::tenant::config::{EvictionPolicy, TenantConfOpt}; -use pageserver_api::reltag::RelTag; +use pageserver_api::reltag::{BlockNumber, RelTag}; use pageserver_api::shard::ShardIndex; use postgres_connection::PgConnectionConfig; diff --git a/pageserver/src/walingest.rs b/pageserver/src/walingest.rs index 8df0c81c7a..852b290f77 100644 --- a/pageserver/src/walingest.rs +++ b/pageserver/src/walingest.rs @@ -38,7 +38,7 @@ use crate::tenant::PageReconstructError; use crate::tenant::Timeline; use crate::walrecord::*; use crate::ZERO_PAGE; -use pageserver_api::reltag::{RelTag, SlruKind}; +use pageserver_api::reltag::{BlockNumber, RelTag, SlruKind}; use postgres_ffi::pg_constants; use postgres_ffi::relfile_utils::{FSM_FORKNUM, INIT_FORKNUM, MAIN_FORKNUM, VISIBILITYMAP_FORKNUM}; use postgres_ffi::v14::nonrelfile_utils::mx_offset_to_member_segment; diff --git a/pageserver/src/walredo.rs b/pageserver/src/walredo.rs index 6918698f29..b4aadb2a8c 100644 --- a/pageserver/src/walredo.rs +++ b/pageserver/src/walredo.rs @@ -47,9 +47,11 @@ use crate::metrics::{ WAL_REDO_PROCESS_LAUNCH_DURATION_HISTOGRAM, WAL_REDO_RECORDS_HISTOGRAM, WAL_REDO_RECORD_COUNTER, WAL_REDO_TIME, }; -use crate::pgdatadir_mapping::{key_to_rel_block, key_to_slru_block}; +use crate::pgdatadir_mapping::key_to_slru_block; use crate::repository::Key; use crate::walrecord::NeonWalRecord; + +use pageserver_api::key::key_to_rel_block; use pageserver_api::reltag::{RelTag, SlruKind}; use postgres_ffi::pg_constants; use postgres_ffi::relfile_utils::VISIBILITYMAP_FORKNUM;