diff --git a/pageserver/src/layered_repository/timeline.rs b/pageserver/src/layered_repository/timeline.rs index da3a6981da..1b77f1fab4 100644 --- a/pageserver/src/layered_repository/timeline.rs +++ b/pageserver/src/layered_repository/timeline.rs @@ -51,7 +51,7 @@ use utils::{ zid::{ZTenantId, ZTimelineId}, }; -use crate::repository::{GcResult, RepositoryTimeline, TimelineWriter}; +use crate::repository::{GcResult, RepositoryTimeline}; use crate::repository::{Key, Value}; use crate::thread_mgr; use crate::virtual_file::VirtualFile; @@ -597,12 +597,11 @@ impl Timeline { /// FIXME: This ought to return &'a TimelineWriter, where TimelineWriter /// is a generic type in this trait. But that doesn't currently work in /// Rust: https://rust-lang.github.io/rfcs/1598-generic_associated_types.html - /// TODO kb replace with the concrete type - pub fn writer<'a>(&'a self) -> Box { - Box::new(LayeredTimelineWriter { + pub fn writer(&self) -> TimelineWriter<'_> { + TimelineWriter { tl: self, _write_guard: self.write_lock.lock().unwrap(), - }) + } } } @@ -2204,12 +2203,16 @@ fn layer_traversal_error( Err(msg_iter.fold(err, |err, msg| err.context(msg))) } -struct LayeredTimelineWriter<'a> { +/// Various functions to mutate the timeline. +// TODO Currently, Deref is used to allow easy access to read methods from this trait. +// This is probably considered a bad practice in Rust and should be fixed eventually, +// but will cause large code changes. +pub struct TimelineWriter<'a> { tl: &'a Timeline, _write_guard: MutexGuard<'a, ()>, } -impl Deref for LayeredTimelineWriter<'_> { +impl Deref for TimelineWriter<'_> { type Target = Timeline; fn deref(&self) -> &Self::Target { @@ -2217,23 +2220,32 @@ impl Deref for LayeredTimelineWriter<'_> { } } -impl<'a> TimelineWriter<'_> for LayeredTimelineWriter<'a> { - fn put(&self, key: Key, lsn: Lsn, value: &Value) -> Result<()> { +impl<'a> TimelineWriter<'a> { + /// Put a new page version that can be constructed from a WAL record + /// + /// This will implicitly extend the relation, if the page is beyond the + /// current end-of-file. + pub fn put(&self, key: Key, lsn: Lsn, value: &Value) -> Result<()> { self.tl.put_value(key, lsn, value) } - fn delete(&self, key_range: Range, lsn: Lsn) -> Result<()> { + pub fn delete(&self, key_range: Range, lsn: Lsn) -> Result<()> { self.tl.put_tombstone(key_range, lsn) } - /// + /// Track the end of the latest digested WAL record. /// Remember the (end of) last valid WAL record remembered in the timeline. /// - fn finish_write(&self, new_lsn: Lsn) { + /// Call this after you have finished writing all the WAL up to 'lsn'. + /// + /// 'lsn' must be aligned. This wakes up any wait_lsn() callers waiting for + /// the 'lsn' or anything older. The previous last record LSN is stored alongside + /// the latest and can be read. + pub fn finish_write(&self, new_lsn: Lsn) { self.tl.finish_write(new_lsn); } - fn update_current_logical_size(&self, delta: isize) { + pub fn update_current_logical_size(&self, delta: isize) { self.tl .current_logical_size .fetch_add(delta, AtomicOrdering::SeqCst); diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs index d0e1ed24b6..dc031c03ee 100644 --- a/pageserver/src/repository.rs +++ b/pageserver/src/repository.rs @@ -8,7 +8,6 @@ use std::fmt; use std::ops::{AddAssign, Range}; use std::sync::Arc; use std::time::Duration; -use utils::lsn::Lsn; #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize, Deserialize)] /// Key used in the Repository kv-store. @@ -228,37 +227,13 @@ impl AddAssign for GcResult { } } -/// Various functions to mutate the timeline. -// TODO Currently, Deref is used to allow easy access to read methods from this trait. -// This is probably considered a bad practice in Rust and should be fixed eventually, -// but will cause large code changes. -pub trait TimelineWriter<'a> { - /// Put a new page version that can be constructed from a WAL record - /// - /// This will implicitly extend the relation, if the page is beyond the - /// current end-of-file. - fn put(&self, key: Key, lsn: Lsn, value: &Value) -> Result<()>; - - fn delete(&self, key_range: Range, lsn: Lsn) -> Result<()>; - - /// Track the end of the latest digested WAL record. - /// - /// Call this after you have finished writing all the WAL up to 'lsn'. - /// - /// 'lsn' must be aligned. This wakes up any wait_lsn() callers waiting for - /// the 'lsn' or anything older. The previous last record LSN is stored alongside - /// the latest and can be read. - fn finish_write(&self, lsn: Lsn); - - fn update_current_logical_size(&self, delta: isize); -} - #[cfg(test)] pub mod repo_harness { use bytes::BytesMut; use once_cell::sync::Lazy; use std::sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard}; use std::{fs, path::PathBuf}; + use utils::lsn::Lsn; use crate::storage_sync::index::RemoteIndex; use crate::{ @@ -440,6 +415,7 @@ mod tests { use bytes::BytesMut; use hex_literal::hex; use once_cell::sync::Lazy; + use utils::lsn::Lsn; static TEST_KEY: Lazy = Lazy::new(|| Key::from_slice(&hex!("112222222233333333444444445500000001")));