diff --git a/pageserver/src/basebackup.rs b/pageserver/src/basebackup.rs index 1ee48eb2fc..c316fc43d1 100644 --- a/pageserver/src/basebackup.rs +++ b/pageserver/src/basebackup.rs @@ -145,16 +145,17 @@ impl<'a> Basebackup<'a> { .timeline .get_relish_size(RelishTag::Slru { slru, segno }, self.lsn)?; - if seg_size == None { - trace!( - "SLRU segment {}/{:>04X} was truncated", - slru.to_str(), - segno - ); - return Ok(()); - } - - let nblocks = seg_size.unwrap(); + let nblocks = match seg_size { + Some(seg_size) => seg_size, + None => { + trace!( + "SLRU segment {}/{:>04X} was truncated", + slru.to_str(), + segno + ); + return Ok(()); + } + }; let mut slru_buf: Vec = Vec::with_capacity(nblocks as usize * pg_constants::BLCKSZ as usize); diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index a2564d51d7..5a1b5e5e2c 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -30,7 +30,7 @@ use zenith_utils::postgres_backend; use zenith_utils::shutdown::exit_now; use zenith_utils::signals::{self, Signal}; -fn main() -> Result<()> { +fn main() -> anyhow::Result<()> { zenith_metrics::set_common_metrics_prefix("pageserver"); let arg_matches = App::new("Zenith page server") .about("Materializes WAL stream to pages and serves them to the postgres") @@ -116,7 +116,7 @@ fn main() -> Result<()> { // We're initializing the repo, so there's no config file yet DEFAULT_CONFIG_FILE .parse::() - .expect("could not parse built-in config file") + .context("could not parse built-in config file")? } else { // Supplement the CLI arguments with the config file let cfg_file_contents = std::fs::read_to_string(&cfg_file_path) @@ -209,7 +209,9 @@ fn start_pageserver(conf: &'static PageServerConf, daemonize: bool) -> Result<() // There shouldn't be any logging to stdin/stdout. Redirect it to the main log so // that we will see any accidental manual fprintf's or backtraces. - let stdout = log_file.try_clone().unwrap(); + let stdout = log_file + .try_clone() + .with_context(|| format!("Failed to clone log file '{:?}'", log_file))?; let stderr = log_file; let daemonize = Daemonize::new() diff --git a/pageserver/src/import_datadir.rs b/pageserver/src/import_datadir.rs index e317118bb5..1e691fb2fe 100644 --- a/pageserver/src/import_datadir.rs +++ b/pageserver/src/import_datadir.rs @@ -70,11 +70,11 @@ pub fn import_timeline_from_postgres_datadir( let direntry = direntry?; //skip all temporary files - if direntry.file_name().to_str().unwrap() == "pgsql_tmp" { + if direntry.file_name().to_string_lossy() == "pgsql_tmp" { continue; } - let dboid = direntry.file_name().to_str().unwrap().parse::()?; + let dboid = direntry.file_name().to_string_lossy().parse::()?; for direntry in fs::read_dir(direntry.path())? { let direntry = direntry?; @@ -117,7 +117,7 @@ pub fn import_timeline_from_postgres_datadir( } for entry in fs::read_dir(path.join("pg_twophase"))? { let entry = entry?; - let xid = u32::from_str_radix(entry.path().to_str().unwrap(), 16)?; + let xid = u32::from_str_radix(&entry.path().to_string_lossy(), 16)?; import_nonrel_file(writer, lsn, RelishTag::TwoPhase { xid }, &entry.path())?; } // TODO: Scan pg_tblspc @@ -156,16 +156,15 @@ fn import_relfile( lsn: Lsn, spcoid: Oid, dboid: Oid, -) -> Result<()> { +) -> anyhow::Result<()> { // Does it look like a relation file? trace!("importing rel file {}", path.display()); - let p = parse_relfilename(path.file_name().unwrap().to_str().unwrap()); - if let Err(e) = p { - warn!("unrecognized file in postgres datadir: {:?} ({})", path, e); - return Err(e.into()); - } - let (relnode, forknum, segno) = p.unwrap(); + let (relnode, forknum, segno) = parse_relfilename(&path.file_name().unwrap().to_string_lossy()) + .map_err(|e| { + warn!("unrecognized file in postgres datadir: {:?} ({})", path, e); + e + })?; let mut file = File::open(path)?; let mut buf: [u8; 8192] = [0u8; 8192]; @@ -271,7 +270,7 @@ fn import_slru_file( // Does it look like an SLRU file? let mut file = File::open(path)?; let mut buf: [u8; 8192] = [0u8; 8192]; - let segno = u32::from_str_radix(path.file_name().unwrap().to_str().unwrap(), 16)?; + let segno = u32::from_str_radix(&path.file_name().unwrap().to_string_lossy(), 16)?; trace!("importing slru file {}", path.display()); diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs index 9cb0a17e66..4d8d0ada24 100644 --- a/pageserver/src/layered_repository.rs +++ b/pageserver/src/layered_repository.rs @@ -11,7 +11,7 @@ //! parent timeline, and the last LSN that has been written to disk. //! -use anyhow::{bail, ensure, Context, Result}; +use anyhow::{anyhow, bail, ensure, Context, Result}; use bookfile::Book; use bytes::Bytes; use lazy_static::lazy_static; @@ -1157,9 +1157,9 @@ impl LayeredTimeline { for direntry in fs::read_dir(timeline_path)? { let direntry = direntry?; let fname = direntry.file_name(); - let fname = fname.to_str().unwrap(); + let fname = fname.to_string_lossy(); - if let Some(imgfilename) = ImageFileName::parse_str(fname) { + if let Some(imgfilename) = ImageFileName::parse_str(&fname) { // create an ImageLayer struct for each image file. if imgfilename.lsn > disk_consistent_lsn { warn!( @@ -1177,7 +1177,7 @@ impl LayeredTimeline { trace!("found layer {}", layer.filename().display()); layers.insert_historic(Arc::new(layer)); num_layers += 1; - } else if let Some(deltafilename) = DeltaFileName::parse_str(fname) { + } else if let Some(deltafilename) = DeltaFileName::parse_str(&fname) { // Create a DeltaLayer struct for each delta file. ensure!(deltafilename.start_lsn < deltafilename.end_lsn); // The end-LSN is exclusive, while disk_consistent_lsn is @@ -1203,7 +1203,7 @@ impl LayeredTimeline { num_layers += 1; } else if fname == METADATA_FILE_NAME || fname.ends_with(".old") { // ignore these - } else if is_ephemeral_file(fname) { + } else if is_ephemeral_file(&fname) { // Delete any old ephemeral files trace!("deleting old ephemeral file in timeline dir: {}", fname); fs::remove_file(direntry.path())?; @@ -1938,7 +1938,7 @@ impl LayeredTimeline { seg_blknum: SegmentBlk, lsn: Lsn, layer: &dyn Layer, - ) -> Result { + ) -> anyhow::Result { // Check the page cache. We will get back the most recent page with lsn <= `lsn`. // The cached image can be returned directly if there is no WAL between the cached image // and requested LSN. The cached image can also be used to reduce the amount of WAL needed @@ -1950,7 +1950,9 @@ impl LayeredTimeline { match cached_lsn.cmp(&lsn) { cmp::Ordering::Less => {} // there might be WAL between cached_lsn and lsn, we need to check cmp::Ordering::Equal => return Ok(cached_img), // exact LSN match, return the image - cmp::Ordering::Greater => panic!(), // the returned lsn should never be after the requested lsn + cmp::Ordering::Greater => { + bail!("the returned lsn should never be after the requested lsn") + } } Some((cached_lsn, cached_img)) } @@ -2341,7 +2343,10 @@ pub fn dump_layerfile_from_path(path: &Path) -> Result<()> { /// Add a suffix to a layer file's name: .{num}.old /// Uses the first available num (starts at 0) fn rename_to_backup(path: PathBuf) -> anyhow::Result<()> { - let filename = path.file_name().unwrap().to_str().unwrap(); + let filename = path + .file_name() + .ok_or_else(|| anyhow!("Path {} don't have a file name", path.display()))? + .to_string_lossy(); let mut new_path = path.clone(); for i in 0u32.. { diff --git a/pageserver/src/layered_repository/inmemory_layer.rs b/pageserver/src/layered_repository/inmemory_layer.rs index 6e24bf6022..239fb341a5 100644 --- a/pageserver/src/layered_repository/inmemory_layer.rs +++ b/pageserver/src/layered_repository/inmemory_layer.rs @@ -17,7 +17,7 @@ use crate::layered_repository::LayeredTimeline; use crate::layered_repository::ZERO_PAGE; use crate::repository::ZenithWalRecord; use crate::{ZTenantId, ZTimelineId}; -use anyhow::{ensure, Result}; +use anyhow::{ensure, Result, bail}; use bytes::Bytes; use log::*; use std::collections::HashMap; @@ -150,9 +150,9 @@ impl InMemoryLayerInner { let pos = self.file.stream_position()?; // make room for the 'length' field by writing zeros as a placeholder. - self.file.seek(std::io::SeekFrom::Start(pos + 4)).unwrap(); + self.file.seek(std::io::SeekFrom::Start(pos + 4))?; - pv.ser_into(&mut self.file).unwrap(); + pv.ser_into(&mut self.file)?; // write the 'length' field. let len = self.file.stream_position()? - pos - 4; @@ -315,7 +315,7 @@ impl Layer for InMemoryLayer { return Ok(false); } } else { - panic!("dropped in-memory layer with no end LSN"); + bail!("dropped in-memory layer with no end LSN"); } } @@ -333,7 +333,7 @@ impl Layer for InMemoryLayer { /// Nothing to do here. When you drop the last reference to the layer, it will /// be deallocated. fn delete(&self) -> Result<()> { - panic!("can't delete an InMemoryLayer") + bail!("can't delete an InMemoryLayer") } fn is_incremental(&self) -> bool { diff --git a/pageserver/src/page_cache.rs b/pageserver/src/page_cache.rs index 2992d9477b..ef802ba0e2 100644 --- a/pageserver/src/page_cache.rs +++ b/pageserver/src/page_cache.rs @@ -732,9 +732,10 @@ impl PageCache { CacheKey::MaterializedPage { hash_key: _, lsn: _, - } => { - panic!("unexpected dirty materialized page"); - } + } => Err(std::io::Error::new( + std::io::ErrorKind::Other, + "unexpected dirty materialized page", + )), CacheKey::EphemeralPage { file_id, blkno } => { writeback_ephemeral_file(*file_id, *blkno, buf) } diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index 6e6b6415f3..6acdc8e93d 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -574,7 +574,6 @@ impl postgres_backend::Handler for PageServerHandler { let data = self .auth .as_ref() - .as_ref() .unwrap() .decode(str::from_utf8(jwt_response)?)?; diff --git a/pageserver/src/tenant_threads.rs b/pageserver/src/tenant_threads.rs index 062af9f1ad..c370eb61c8 100644 --- a/pageserver/src/tenant_threads.rs +++ b/pageserver/src/tenant_threads.rs @@ -49,7 +49,7 @@ pub fn gc_loop(tenantid: ZTenantId, conf: &'static PageServerConf) -> Result<()> // Garbage collect old files that are not needed for PITR anymore if conf.gc_horizon > 0 { let repo = tenant_mgr::get_repository_for_tenant(tenantid)?; - repo.gc_iteration(None, conf.gc_horizon, false).unwrap(); + repo.gc_iteration(None, conf.gc_horizon, false)?; } // TODO Write it in more adequate way using diff --git a/pageserver/src/thread_mgr.rs b/pageserver/src/thread_mgr.rs index a51f0909ca..d24d6bf016 100644 --- a/pageserver/src/thread_mgr.rs +++ b/pageserver/src/thread_mgr.rs @@ -250,7 +250,7 @@ pub fn shutdown_threads( let _ = join_handle.join(); } else { // The thread had not even fully started yet. Or it was shut down - // concurrently and alrady exited + // concurrently and already exited } } } diff --git a/pageserver/src/timelines.rs b/pageserver/src/timelines.rs index 00dd0f8f9c..8c018ce70f 100644 --- a/pageserver/src/timelines.rs +++ b/pageserver/src/timelines.rs @@ -250,7 +250,7 @@ fn run_initdb(conf: &'static PageServerConf, initdbpath: &Path) -> Result<()> { let initdb_path = conf.pg_bin_dir().join("initdb"); let initdb_output = Command::new(initdb_path) - .args(&["-D", initdbpath.to_str().unwrap()]) + .args(&["-D", &initdbpath.to_string_lossy()]) .args(&["-U", &conf.superuser]) .args(&["-E", "utf8"]) .arg("--no-instructions") @@ -258,8 +258,8 @@ fn run_initdb(conf: &'static PageServerConf, initdbpath: &Path) -> Result<()> { // so no need to fsync it .arg("--no-sync") .env_clear() - .env("LD_LIBRARY_PATH", conf.pg_lib_dir().to_str().unwrap()) - .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir().to_str().unwrap()) + .env("LD_LIBRARY_PATH", conf.pg_lib_dir()) + .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir()) .stdout(Stdio::null()) .output() .context("failed to execute initdb")?; diff --git a/pageserver/src/virtual_file.rs b/pageserver/src/virtual_file.rs index 73671dcf4e..858cff29cb 100644 --- a/pageserver/src/virtual_file.rs +++ b/pageserver/src/virtual_file.rs @@ -226,7 +226,8 @@ impl VirtualFile { path: &Path, open_options: &OpenOptions, ) -> Result { - let parts = path.to_str().unwrap().split('/').collect::>(); + let path_str = path.to_string_lossy(); + let parts = path_str.split('/').collect::>(); let tenantid; let timelineid; if parts.len() > 5 && parts[parts.len() - 5] == "tenants" { diff --git a/pageserver/src/walingest.rs b/pageserver/src/walingest.rs index 1962c9bbd3..506890476f 100644 --- a/pageserver/src/walingest.rs +++ b/pageserver/src/walingest.rs @@ -249,7 +249,7 @@ impl WalIngest { { let mut checkpoint_bytes = [0u8; SIZEOF_CHECKPOINT]; buf.copy_to_slice(&mut checkpoint_bytes); - let xlog_checkpoint = CheckPoint::decode(&checkpoint_bytes).unwrap(); + let xlog_checkpoint = CheckPoint::decode(&checkpoint_bytes)?; trace!( "xlog_checkpoint.oldestXid={}, checkpoint.oldestXid={}", xlog_checkpoint.oldestXid, diff --git a/pageserver/src/walredo.rs b/pageserver/src/walredo.rs index 877b81b8d5..704b8f2583 100644 --- a/pageserver/src/walredo.rs +++ b/pageserver/src/walredo.rs @@ -375,7 +375,10 @@ impl PostgresRedoManager { ZenithWalRecord::Postgres { will_init: _, rec: _, - } => panic!("tried to pass postgres wal record to zenith WAL redo"), + } => { + error!("tried to pass postgres wal record to zenith WAL redo"); + return Err(WalRedoError::InvalidRequest); + } ZenithWalRecord::ClearVisibilityMapFlags { new_heap_blkno, old_heap_blkno, @@ -541,20 +544,23 @@ impl PostgresRedoProcess { } info!("running initdb in {:?}", datadir.display()); let initdb = Command::new(conf.pg_bin_dir().join("initdb")) - .args(&["-D", datadir.to_str().unwrap()]) + .args(&["-D", &datadir.to_string_lossy()]) .arg("-N") .env_clear() - .env("LD_LIBRARY_PATH", conf.pg_lib_dir().to_str().unwrap()) - .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir().to_str().unwrap()) + .env("LD_LIBRARY_PATH", conf.pg_lib_dir()) + .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir()) .output() - .expect("failed to execute initdb"); + .map_err(|e| Error::new(e.kind(), format!("failed to execute initdb: {}", e)))?; if !initdb.status.success() { - panic!( - "initdb failed: {}\nstderr:\n{}", - std::str::from_utf8(&initdb.stdout).unwrap(), - std::str::from_utf8(&initdb.stderr).unwrap() - ); + return Err(Error::new( + ErrorKind::Other, + format!( + "initdb failed\nstdout: {}\nstderr:\n{}", + String::from_utf8_lossy(&initdb.stdout), + String::from_utf8_lossy(&initdb.stderr) + ), + )); } else { // Limit shared cache for wal-redo-postres let mut config = OpenOptions::new() @@ -572,11 +578,16 @@ impl PostgresRedoProcess { .stderr(Stdio::piped()) .stdout(Stdio::piped()) .env_clear() - .env("LD_LIBRARY_PATH", conf.pg_lib_dir().to_str().unwrap()) - .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir().to_str().unwrap()) + .env("LD_LIBRARY_PATH", conf.pg_lib_dir()) + .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir()) .env("PGDATA", &datadir) .spawn() - .expect("postgres --wal-redo command failed to start"); + .map_err(|e| { + Error::new( + e.kind(), + format!("postgres --wal-redo command failed to start: {}", e), + ) + })?; info!( "launched WAL redo postgres process on {:?}", @@ -636,7 +647,10 @@ impl PostgresRedoProcess { { build_apply_record_msg(*lsn, postgres_rec, &mut writebuf); } else { - panic!("tried to pass zenith wal record to postgres WAL redo"); + return Err(Error::new( + ErrorKind::Other, + "tried to pass zenith wal record to postgres WAL redo", + )); } } build_get_page_msg(tag, &mut writebuf);