From b7575582b84c6e4cd2bec038f38c4e20c4098269 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 6 May 2021 23:02:11 +0300 Subject: [PATCH] Add comments to the Repository/Timeline traits. Let's try to have comments on every public function. This doesn't quite get us there yet, but close. --- pageserver/src/repository.rs | 52 +++++++++++++++++++++++++--- pageserver/src/repository/rocksdb.rs | 4 +-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs index 2b41363ff9..81fc3a4536 100644 --- a/pageserver/src/repository.rs +++ b/pageserver/src/repository.rs @@ -11,9 +11,17 @@ use zenith_utils::lsn::Lsn; /// A repository corresponds to one .zenith directory. One repository holds multiple /// timelines, forked off from the same initial call to 'initdb'. /// +/// FIXME: I wish these would return an abstract `&dyn Timeline`. pub trait Repository { - // FIXME: I wish these would return an abstract `&dyn Timeline` instead + /// Get Timeline handle for given zenith timeline ID. + /// + /// The Timeline is expected to be already "open", i.e. `get_or_restore_timeline` + /// should've been called on it earlier already. fn get_timeline(&self, timelineid: ZTimelineId) -> Result>; + + /// Get Timeline handle for given zenith timeline ID. + /// + /// Creates a new Timeline object if it's not "open" already. fn get_or_restore_timeline( &self, timelineid: ZTimelineId, @@ -24,6 +32,10 @@ pub trait Repository { pub trait Timeline { + //------------------------------------------------------------------------------ + // Public GET functions + //------------------------------------------------------------------------------ + /// Look up given page in the cache. fn get_page_at_lsn(&self, tag: BufferTag, lsn: Lsn) -> Result; @@ -33,12 +45,30 @@ pub trait Timeline { /// Does relation exist? fn get_relsize_exists(&self, tag: RelTag, lsn: Lsn) -> Result; - // Functions used by WAL receiver + //------------------------------------------------------------------------------ + // Public PUT functions, to update the repository with new page versions. + // + // These are called by the WAL receiver to digest WAL records. + //------------------------------------------------------------------------------ + /// 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_wal_record(&self, tag: BufferTag, rec: WALRecord); - fn put_truncation(&self, rel: RelTag, lsn: Lsn, nblocks: u32) -> anyhow::Result<()>; + + /// Like put_wal_record, but with ready-made image of the page. fn put_page_image(&self, tag: BufferTag, lsn: Lsn, img: Bytes); - fn create_database( + + /// Truncate relation + fn put_truncation(&self, rel: RelTag, lsn: Lsn, nblocks: u32) -> anyhow::Result<()>; + + /// Create a new database from a template database + /// + /// In PostgreSQL, CREATE DATABASE works by scanning the data directory and + /// copying all relation files from the template database. This is the equivalent + /// of that. + fn put_create_database( &self, lsn: Lsn, db_id: Oid, @@ -47,6 +77,10 @@ pub trait Timeline { src_tablespace_id: Oid, ) -> Result<()>; + /// + /// Helper function to parse a WAL record and call the above functions for all the + /// relations/pages that the record affects. + /// fn save_decoded_record( &self, decoded: DecodedWALRecord, @@ -54,10 +88,20 @@ pub trait Timeline { lsn: Lsn ) -> anyhow::Result<()>; + /// Remember the all WAL before the given LSN has been processed. + /// + /// The WAL receiver calls this after the put_* functions, to indicate that + /// all WAL before this point has been digested. Before that, if you call + /// GET on an earlier LSN, it will block. fn advance_last_valid_lsn(&self, lsn: Lsn); fn get_last_valid_lsn(&self) -> Lsn; fn init_valid_lsn(&self, lsn: Lsn); + /// Like `advance_last_valid_lsn`, but this always points to the end of + /// a WAL record, not in the middle of one. + /// + /// This must be <= last valid LSN. This is tracked separately from last + /// valid LSN, so that the WAL receiver knows where to restart streaming. fn advance_last_record_lsn(&self, lsn: Lsn); fn get_last_record_lsn(&self) -> Lsn; } diff --git a/pageserver/src/repository/rocksdb.rs b/pageserver/src/repository/rocksdb.rs index 4558b3e9eb..244da9f411 100644 --- a/pageserver/src/repository/rocksdb.rs +++ b/pageserver/src/repository/rocksdb.rs @@ -745,7 +745,7 @@ impl Timeline for RocksTimeline { self.num_page_images.fetch_add(1, Ordering::Relaxed); } - fn create_database( + fn put_create_database( &self, lsn: Lsn, db_id: Oid, @@ -844,7 +844,7 @@ impl Timeline for RocksTimeline { == pg_constants::XLOG_DBASE_CREATE { let createdb = XlCreateDatabase::decode(&decoded); - self.create_database( + self.put_create_database( lsn, createdb.db_id, createdb.tablespace_id,