From f51b48fa492dd9ff65ac5026c2b6928b41c4e25a Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 24 Feb 2023 13:45:32 +0200 Subject: [PATCH] Fix UNLOGGED tables. Instead of trying to create missing files on the way, send init fork contents as main fork from pageserver during basebackup. Add test for that. Call put_rel_drop for init forks; previously they weren't removed. Bump vendor/postgres to revert previous approach on Postgres side. Co-authored-by: Arseny Sher ref https://github.com/neondatabase/postgres/pull/264 ref https://github.com/neondatabase/postgres/pull/259 ref https://github.com/neondatabase/neon/issues/1222 --- libs/pageserver_api/src/reltag.rs | 9 ++++++ pageserver/src/basebackup.rs | 45 ++++++++++++++++++++-------- pageserver/src/walingest.rs | 4 +-- test_runner/regress/test_unlogged.py | 34 +++++++++++++++++++++ vendor/postgres-v14 | 2 +- vendor/postgres-v15 | 2 +- 6 files changed, 79 insertions(+), 17 deletions(-) create mode 100644 test_runner/regress/test_unlogged.py diff --git a/libs/pageserver_api/src/reltag.rs b/libs/pageserver_api/src/reltag.rs index 43d38bd986..12693379f5 100644 --- a/libs/pageserver_api/src/reltag.rs +++ b/libs/pageserver_api/src/reltag.rs @@ -98,6 +98,15 @@ impl RelTag { name } + + pub fn with_forknum(&self, forknum: u8) -> Self { + RelTag { + forknum, + spcnode: self.spcnode, + dbnode: self.dbnode, + relnode: self.relnode, + } + } } /// diff --git a/pageserver/src/basebackup.rs b/pageserver/src/basebackup.rs index 06d4853274..41fa0a67bb 100644 --- a/pageserver/src/basebackup.rs +++ b/pageserver/src/basebackup.rs @@ -33,6 +33,7 @@ use pageserver_api::reltag::{RelTag, SlruKind}; use postgres_ffi::pg_constants::{DEFAULTTABLESPACE_OID, GLOBALTABLESPACE_OID}; use postgres_ffi::pg_constants::{PGDATA_SPECIAL_FILES, PGDATA_SUBDIRS, PG_HBA}; +use postgres_ffi::relfile_utils::{INIT_FORKNUM, MAIN_FORKNUM}; use postgres_ffi::TransactionId; use postgres_ffi::XLogFileName; use postgres_ffi::PG_TLI; @@ -190,14 +191,31 @@ where { self.add_dbdir(spcnode, dbnode, has_relmap_file).await?; - // Gather and send relational files in each database if full backup is requested. - if self.full_backup { - for rel in self - .timeline - .list_rels(spcnode, dbnode, self.lsn, self.ctx) - .await? - { - self.add_rel(rel).await?; + // If full backup is requested, include all relation files. + // Otherwise only include init forks of unlogged relations. + let rels = self + .timeline + .list_rels(spcnode, dbnode, self.lsn, self.ctx) + .await?; + for &rel in rels.iter() { + // Send init fork as main fork to provide well formed empty + // contents of UNLOGGED relations. Postgres copies it in + // `reinit.c` during recovery. + if rel.forknum == INIT_FORKNUM { + // I doubt we need _init fork itself, but having it at least + // serves as a marker relation is unlogged. + self.add_rel(rel, rel).await?; + self.add_rel(rel, rel.with_forknum(MAIN_FORKNUM)).await?; + continue; + } + + if self.full_backup { + if rel.forknum == MAIN_FORKNUM && rels.contains(&rel.with_forknum(INIT_FORKNUM)) + { + // skip this, will include it when we reach the init fork + continue; + } + self.add_rel(rel, rel).await?; } } } @@ -220,15 +238,16 @@ where Ok(()) } - async fn add_rel(&mut self, tag: RelTag) -> anyhow::Result<()> { + /// Add contents of relfilenode `src`, naming it as `dst`. + async fn add_rel(&mut self, src: RelTag, dst: RelTag) -> anyhow::Result<()> { let nblocks = self .timeline - .get_rel_size(tag, self.lsn, false, self.ctx) + .get_rel_size(src, self.lsn, false, self.ctx) .await?; // If the relation is empty, create an empty file if nblocks == 0 { - let file_name = tag.to_segfile_name(0); + let file_name = dst.to_segfile_name(0); let header = new_tar_header(&file_name, 0)?; self.ar.append(&header, &mut io::empty()).await?; return Ok(()); @@ -244,12 +263,12 @@ where for blknum in startblk..endblk { let img = self .timeline - .get_rel_page_at_lsn(tag, blknum, self.lsn, false, self.ctx) + .get_rel_page_at_lsn(src, blknum, self.lsn, false, self.ctx) .await?; segment_data.extend_from_slice(&img[..]); } - let file_name = tag.to_segfile_name(seg as u32); + let file_name = dst.to_segfile_name(seg as u32); let header = new_tar_header(&file_name, segment_data.len() as u64)?; self.ar.append(&header, segment_data.as_slice()).await?; diff --git a/pageserver/src/walingest.rs b/pageserver/src/walingest.rs index 3761c65668..63d568a342 100644 --- a/pageserver/src/walingest.rs +++ b/pageserver/src/walingest.rs @@ -37,7 +37,7 @@ use crate::walrecord::*; use crate::ZERO_PAGE; use pageserver_api::reltag::{RelTag, SlruKind}; use postgres_ffi::pg_constants; -use postgres_ffi::relfile_utils::{FSM_FORKNUM, MAIN_FORKNUM, VISIBILITYMAP_FORKNUM}; +use postgres_ffi::relfile_utils::{FSM_FORKNUM, INIT_FORKNUM, MAIN_FORKNUM, VISIBILITYMAP_FORKNUM}; use postgres_ffi::v14::nonrelfile_utils::mx_offset_to_member_segment; use postgres_ffi::v14::xlog_utils::*; use postgres_ffi::v14::CheckPoint; @@ -762,7 +762,7 @@ impl<'a> WalIngest<'a> { )?; for xnode in &parsed.xnodes { - for forknum in MAIN_FORKNUM..=VISIBILITYMAP_FORKNUM { + for forknum in MAIN_FORKNUM..=INIT_FORKNUM { let rel = RelTag { forknum, spcnode: xnode.spcnode, diff --git a/test_runner/regress/test_unlogged.py b/test_runner/regress/test_unlogged.py new file mode 100644 index 0000000000..b6b20f1230 --- /dev/null +++ b/test_runner/regress/test_unlogged.py @@ -0,0 +1,34 @@ +from fixtures.neon_fixtures import NeonEnv, fork_at_current_lsn + + +# +# Test UNLOGGED tables/relations. Postgres copies init fork contents to main +# fork to reset them during recovery. In Neon, pageserver directly sends init +# fork contents as main fork during basebackup. +# +def test_unlogged(neon_simple_env: NeonEnv): + env = neon_simple_env + env.neon_cli.create_branch("test_unlogged", "empty") + pg = env.postgres.create_start("test_unlogged") + + conn = pg.connect() + cur = conn.cursor() + + cur.execute("CREATE UNLOGGED TABLE iut (id int);") + # create index to test unlogged index relation as well + cur.execute("CREATE UNIQUE INDEX iut_idx ON iut (id);") + cur.execute("INSERT INTO iut values (42);") + + # create another compute to fetch inital empty contents from pageserver + fork_at_current_lsn(env, pg, "test_unlogged_basebackup", "test_unlogged") + pg2 = env.postgres.create_start( + "test_unlogged_basebackup", + ) + + conn2 = pg2.connect() + cur2 = conn2.cursor() + # after restart table should be empty but valid + cur2.execute("PREPARE iut_plan (int) AS INSERT INTO iut VALUES ($1)") + cur2.execute("EXECUTE iut_plan (43);") + cur2.execute("SELECT * FROM iut") + assert cur2.fetchall() == [(43,)] diff --git a/vendor/postgres-v14 b/vendor/postgres-v14 index 468d3c0824..5fb2e0bba0 160000 --- a/vendor/postgres-v14 +++ b/vendor/postgres-v14 @@ -1 +1 @@ -Subproject commit 468d3c08245906f083fed1009759f9f953f5915d +Subproject commit 5fb2e0bba06cc018ee2506f337c91751ab695454 diff --git a/vendor/postgres-v15 b/vendor/postgres-v15 index 9a2093383a..919851e781 160000 --- a/vendor/postgres-v15 +++ b/vendor/postgres-v15 @@ -1 +1 @@ -Subproject commit 9a2093383ae19906f025b008ceecf89ebc9ea869 +Subproject commit 919851e7811fcb2ecfc67f35bfd63a35639c73b5