From 994411f5c2ada300d71938ca1e5a95db2cbda43a Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Wed, 16 Aug 2023 18:33:47 +0200 Subject: [PATCH] page cache: newtype the blob_io and ephemeral_file file ids (#5005) This makes it more explicit that these are different u64-sized namespaces. Re-using one in place of the other would be catastrophic. Prep for https://github.com/neondatabase/neon/pull/4994 which will eliminate the ephemeral_file::FileId and move the blob_io::FileId into page_cache. It makes sense to have this preliminary commit though, to minimize amount of new concept in #4994 and other preliminaries that depend on that work. --- pageserver/src/page_cache.rs | 32 +++++++++++++++++-------- pageserver/src/tenant/block_io.rs | 10 ++++++-- pageserver/src/tenant/ephemeral_file.rs | 23 ++++++++++++------ 3 files changed, 46 insertions(+), 19 deletions(-) diff --git a/pageserver/src/page_cache.rs b/pageserver/src/page_cache.rs index e29eb1d197..8306ce4636 100644 --- a/pageserver/src/page_cache.rs +++ b/pageserver/src/page_cache.rs @@ -53,7 +53,7 @@ use utils::{ lsn::Lsn, }; -use crate::tenant::writeback_ephemeral_file; +use crate::tenant::{block_io, ephemeral_file, writeback_ephemeral_file}; use crate::{metrics::PageCacheSizeMetrics, repository::Key}; static PAGE_CACHE: OnceCell = OnceCell::new(); @@ -98,11 +98,11 @@ enum CacheKey { lsn: Lsn, }, EphemeralPage { - file_id: u64, + file_id: ephemeral_file::FileId, blkno: u32, }, ImmutableFilePage { - file_id: u64, + file_id: block_io::FileId, blkno: u32, }, } @@ -177,9 +177,9 @@ pub struct PageCache { /// can have a separate mapping map, next to this field. materialized_page_map: RwLock>>, - ephemeral_page_map: RwLock>, + ephemeral_page_map: RwLock>, - immutable_page_map: RwLock>, + immutable_page_map: RwLock>, /// The actual buffers with their metadata. slots: Box<[Slot]>, @@ -390,20 +390,28 @@ impl PageCache { // Section 1.2: Public interface functions for working with Ephemeral pages. - pub fn read_ephemeral_buf(&self, file_id: u64, blkno: u32) -> anyhow::Result { + pub fn read_ephemeral_buf( + &self, + file_id: ephemeral_file::FileId, + blkno: u32, + ) -> anyhow::Result { let mut cache_key = CacheKey::EphemeralPage { file_id, blkno }; self.lock_for_read(&mut cache_key) } - pub fn write_ephemeral_buf(&self, file_id: u64, blkno: u32) -> anyhow::Result { + pub fn write_ephemeral_buf( + &self, + file_id: ephemeral_file::FileId, + blkno: u32, + ) -> anyhow::Result { let cache_key = CacheKey::EphemeralPage { file_id, blkno }; self.lock_for_write(&cache_key) } /// Immediately drop all buffers belonging to given file, without writeback - pub fn drop_buffers_for_ephemeral(&self, drop_file_id: u64) { + pub fn drop_buffers_for_ephemeral(&self, drop_file_id: ephemeral_file::FileId) { for slot_idx in 0..self.slots.len() { let slot = &self.slots[slot_idx]; @@ -424,14 +432,18 @@ impl PageCache { // Section 1.3: Public interface functions for working with immutable file pages. - pub fn read_immutable_buf(&self, file_id: u64, blkno: u32) -> anyhow::Result { + pub fn read_immutable_buf( + &self, + file_id: block_io::FileId, + blkno: u32, + ) -> anyhow::Result { let mut cache_key = CacheKey::ImmutableFilePage { file_id, blkno }; self.lock_for_read(&mut cache_key) } /// Immediately drop all buffers belonging to given file, without writeback - pub fn drop_buffers_for_immutable(&self, drop_file_id: u64) { + pub fn drop_buffers_for_immutable(&self, drop_file_id: block_io::FileId) { for slot_idx in 0..self.slots.len() { let slot = &self.slots[slot_idx]; diff --git a/pageserver/src/tenant/block_io.rs b/pageserver/src/tenant/block_io.rs index f25df324f4..3cc4e61a95 100644 --- a/pageserver/src/tenant/block_io.rs +++ b/pageserver/src/tenant/block_io.rs @@ -117,6 +117,12 @@ where } } static NEXT_ID: AtomicU64 = AtomicU64::new(1); +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct FileId(u64); + +fn next_file_id() -> FileId { + FileId(NEXT_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed)) +} /// An adapter for reading a (virtual) file using the page cache. /// @@ -126,7 +132,7 @@ pub struct FileBlockReader { pub file: F, /// Unique ID of this file, used as key in the page cache. - file_id: u64, + file_id: FileId, } impl FileBlockReader @@ -134,7 +140,7 @@ where F: FileExt, { pub fn new(file: F) -> Self { - let file_id = NEXT_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let file_id = next_file_id(); FileBlockReader { file_id, file } } diff --git a/pageserver/src/tenant/ephemeral_file.rs b/pageserver/src/tenant/ephemeral_file.rs index 6979d19d33..ddce0cb152 100644 --- a/pageserver/src/tenant/ephemeral_file.rs +++ b/pageserver/src/tenant/ephemeral_file.rs @@ -23,19 +23,28 @@ use utils::id::{TenantId, TimelineId}; /// static EPHEMERAL_FILES: Lazy> = Lazy::new(|| { RwLock::new(EphemeralFiles { - next_file_id: 1, + next_file_id: FileId(1), files: HashMap::new(), }) }); -pub struct EphemeralFiles { - next_file_id: u64, +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct FileId(u64); - files: HashMap>, +impl std::fmt::Display for FileId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +pub struct EphemeralFiles { + next_file_id: FileId, + + files: HashMap>, } pub struct EphemeralFile { - file_id: u64, + file_id: FileId, _tenant_id: TenantId, _timeline_id: TimelineId, file: Arc, @@ -51,7 +60,7 @@ impl EphemeralFile { ) -> Result { let mut l = EPHEMERAL_FILES.write().unwrap(); let file_id = l.next_file_id; - l.next_file_id += 1; + l.next_file_id = FileId(l.next_file_id.0 + 1); let filename = conf .timeline_path(&tenant_id, &timeline_id) @@ -211,7 +220,7 @@ impl Drop for EphemeralFile { } } -pub fn writeback(file_id: u64, blkno: u32, buf: &[u8]) -> Result<(), io::Error> { +pub fn writeback(file_id: FileId, blkno: u32, buf: &[u8]) -> Result<(), io::Error> { if let Some(file) = EPHEMERAL_FILES.read().unwrap().files.get(&file_id) { match file.write_all_at(buf, blkno as u64 * PAGE_SZ as u64) { Ok(_) => Ok(()),