diff --git a/Cargo.lock b/Cargo.lock index 3ce0ce465f..fc4ef90b8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2444,6 +2444,7 @@ dependencies = [ "tokio-util", "toml_edit", "tracing", + "utils", "workspace_hack", ] @@ -3929,13 +3930,7 @@ dependencies = [ "chrono", "either", "fail", - "futures-channel", - "futures-task", - "futures-util", - "generic-array", "hashbrown", - "hex", - "hyper", "indexmap", "itoa 0.4.8", "libc", diff --git a/libs/remote_storage/Cargo.toml b/libs/remote_storage/Cargo.toml index b3485f274a..cec344a4ad 100644 --- a/libs/remote_storage/Cargo.toml +++ b/libs/remote_storage/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" anyhow = { version = "1.0", features = ["backtrace"] } async-trait = "0.1" metrics = { version = "0.1", path = "../metrics" } +utils = { version = "0.1", path = "../utils" } once_cell = "1.13.0" rusoto_core = "0.48" rusoto_s3 = "0.48" diff --git a/libs/remote_storage/src/lib.rs b/libs/remote_storage/src/lib.rs index 6b3fd29a0e..4bdd2b9608 100644 --- a/libs/remote_storage/src/lib.rs +++ b/libs/remote_storage/src/lib.rs @@ -9,9 +9,7 @@ mod local_fs; mod s3_bucket; use std::{ - borrow::Cow, collections::HashMap, - ffi::OsStr, fmt::{Debug, Display}, num::{NonZeroU32, NonZeroUsize}, ops::Deref, @@ -344,22 +342,6 @@ impl Debug for S3Config { } } -/// Adds a suffix to the file(directory) name, either appending the suffux to the end of its extension, -/// or if there's no extension, creates one and puts a suffix there. -pub fn path_with_suffix_extension(original_path: impl AsRef, suffix: &str) -> PathBuf { - let new_extension = match original_path - .as_ref() - .extension() - .map(OsStr::to_string_lossy) - { - Some(extension) => Cow::Owned(format!("{extension}.{suffix}")), - None => Cow::Borrowed(suffix), - }; - original_path - .as_ref() - .with_extension(new_extension.as_ref()) -} - impl RemoteStorageConfig { pub fn from_toml(toml: &toml_edit::Item) -> anyhow::Result { let local_path = toml.get("local_path"); @@ -448,35 +430,6 @@ fn parse_toml_string(name: &str, item: &Item) -> anyhow::Result { mod tests { use super::*; - #[test] - fn test_path_with_suffix_extension() { - let p = PathBuf::from("/foo/bar"); - assert_eq!( - &path_with_suffix_extension(&p, "temp").to_string_lossy(), - "/foo/bar.temp" - ); - let p = PathBuf::from("/foo/bar"); - assert_eq!( - &path_with_suffix_extension(&p, "temp.temp").to_string_lossy(), - "/foo/bar.temp.temp" - ); - let p = PathBuf::from("/foo/bar.baz"); - assert_eq!( - &path_with_suffix_extension(&p, "temp.temp").to_string_lossy(), - "/foo/bar.baz.temp.temp" - ); - let p = PathBuf::from("/foo/bar.baz"); - assert_eq!( - &path_with_suffix_extension(&p, ".temp").to_string_lossy(), - "/foo/bar.baz..temp" - ); - let p = PathBuf::from("/foo/bar/dir/"); - assert_eq!( - &path_with_suffix_extension(&p, ".temp").to_string_lossy(), - "/foo/bar/dir..temp" - ); - } - #[test] fn object_name() { let k = RemoteObjectId("a/b/c".to_owned()); diff --git a/libs/remote_storage/src/local_fs.rs b/libs/remote_storage/src/local_fs.rs index 3ffbf3cb39..5723a512f6 100644 --- a/libs/remote_storage/src/local_fs.rs +++ b/libs/remote_storage/src/local_fs.rs @@ -16,8 +16,9 @@ use tokio::{ io::{self, AsyncReadExt, AsyncSeekExt, AsyncWriteExt}, }; use tracing::*; +use utils::crashsafe_dir::path_with_suffix_extension; -use crate::{path_with_suffix_extension, Download, DownloadError, RemoteObjectId}; +use crate::{Download, DownloadError, RemoteObjectId}; use super::{strip_path_prefix, RemoteStorage, StorageMetadata}; diff --git a/libs/utils/src/crashsafe_dir.rs b/libs/utils/src/crashsafe_dir.rs index a7eab73a43..032ab0a916 100644 --- a/libs/utils/src/crashsafe_dir.rs +++ b/libs/utils/src/crashsafe_dir.rs @@ -1,7 +1,9 @@ use std::{ + borrow::Cow, + ffi::OsStr, fs::{self, File}, io, - path::Path, + path::{Path, PathBuf}, }; /// Similar to [`std::fs::create_dir`], except we fsync the @@ -74,6 +76,22 @@ pub fn create_dir_all(path: impl AsRef) -> io::Result<()> { Ok(()) } +/// Adds a suffix to the file(directory) name, either appending the suffix to the end of its extension, +/// or if there's no extension, creates one and puts a suffix there. +pub fn path_with_suffix_extension(original_path: impl AsRef, suffix: &str) -> PathBuf { + let new_extension = match original_path + .as_ref() + .extension() + .map(OsStr::to_string_lossy) + { + Some(extension) => Cow::Owned(format!("{extension}.{suffix}")), + None => Cow::Borrowed(suffix), + }; + original_path + .as_ref() + .with_extension(new_extension.as_ref()) +} + #[cfg(test)] mod tests { use tempfile::tempdir; @@ -122,4 +140,33 @@ mod tests { let invalid_dir_path = file_path.join("folder"); create_dir_all(&invalid_dir_path).unwrap_err(); } + + #[test] + fn test_path_with_suffix_extension() { + let p = PathBuf::from("/foo/bar"); + assert_eq!( + &path_with_suffix_extension(&p, "temp").to_string_lossy(), + "/foo/bar.temp" + ); + let p = PathBuf::from("/foo/bar"); + assert_eq!( + &path_with_suffix_extension(&p, "temp.temp").to_string_lossy(), + "/foo/bar.temp.temp" + ); + let p = PathBuf::from("/foo/bar.baz"); + assert_eq!( + &path_with_suffix_extension(&p, "temp.temp").to_string_lossy(), + "/foo/bar.baz.temp.temp" + ); + let p = PathBuf::from("/foo/bar.baz"); + assert_eq!( + &path_with_suffix_extension(&p, ".temp").to_string_lossy(), + "/foo/bar.baz..temp" + ); + let p = PathBuf::from("/foo/bar/dir/"); + assert_eq!( + &path_with_suffix_extension(&p, ".temp").to_string_lossy(), + "/foo/bar/dir..temp" + ); + } } diff --git a/pageserver/src/storage_sync/download.rs b/pageserver/src/storage_sync/download.rs index 980001f95d..3e850443d8 100644 --- a/pageserver/src/storage_sync/download.rs +++ b/pageserver/src/storage_sync/download.rs @@ -9,7 +9,7 @@ use std::{ use anyhow::Context; use futures::stream::{FuturesUnordered, StreamExt}; -use remote_storage::{path_with_suffix_extension, DownloadError, GenericRemoteStorage}; +use remote_storage::{DownloadError, GenericRemoteStorage}; use tokio::{ fs, io::{self, AsyncWriteExt}, @@ -17,7 +17,10 @@ use tokio::{ use tracing::{debug, error, info, warn}; use crate::{config::PageServerConf, storage_sync::SyncTask, TEMP_FILE_SUFFIX}; -use utils::id::{TenantId, TenantTimelineId, TimelineId}; +use utils::{ + crashsafe_dir::path_with_suffix_extension, + id::{TenantId, TenantTimelineId, TimelineId}, +}; use super::{ index::{IndexPart, RemoteTimeline}, diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 40c9f1e9ad..ca97796870 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -14,6 +14,7 @@ use anyhow::{bail, ensure, Context, Result}; use tokio::sync::watch; use tracing::*; +use utils::crashsafe_dir::path_with_suffix_extension; use std::cmp::min; use std::collections::hash_map; @@ -45,7 +46,6 @@ use crate::tenant_config::TenantConfOpt; use crate::virtual_file::VirtualFile; use crate::walredo::WalRedoManager; use crate::{CheckpointConfig, TEMP_FILE_SUFFIX}; -use remote_storage::path_with_suffix_extension; use toml_edit; use utils::{ @@ -974,10 +974,6 @@ impl Tenant { None }; - // create a new timeline directory - let timelinedir = self.conf.timeline_path(&dst, &self.tenant_id); - crashsafe_dir::create_dir(&timelinedir)?; - // Create the metadata file, noting the ancestor of the new timeline. // There is initially no data in it, but all the read-calls know to look // into the ancestor. diff --git a/pageserver/src/tenant_mgr.rs b/pageserver/src/tenant_mgr.rs index 2c6f5fa863..fcb2c18b79 100644 --- a/pageserver/src/tenant_mgr.rs +++ b/pageserver/src/tenant_mgr.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use anyhow::Context; use tracing::*; -use remote_storage::{path_with_suffix_extension, GenericRemoteStorage}; +use remote_storage::GenericRemoteStorage; use crate::config::{PageServerConf, METADATA_FILE_NAME}; use crate::http::models::TenantInfo; @@ -24,7 +24,7 @@ use crate::tenant_config::TenantConfOpt; use crate::walredo::PostgresRedoManager; use crate::{TenantTimelineValues, TEMP_FILE_SUFFIX}; -use utils::crashsafe_dir; +use utils::crashsafe_dir::{self, path_with_suffix_extension}; use utils::id::{TenantId, TimelineId}; mod tenants_state { diff --git a/pageserver/src/walreceiver/connection_manager.rs b/pageserver/src/walreceiver/connection_manager.rs index 799062e935..148372c9d0 100644 --- a/pageserver/src/walreceiver/connection_manager.rs +++ b/pageserver/src/walreceiver/connection_manager.rs @@ -1358,7 +1358,7 @@ mod tests { const DUMMY_SAFEKEEPER_CONNSTR: &str = "safekeeper_connstr"; - fn dummy_state(harness: &TenantHarness) -> WalreceiverState { + fn dummy_state(harness: &TenantHarness<'_>) -> WalreceiverState { WalreceiverState { id: TenantTimelineId { tenant_id: harness.tenant_id, diff --git a/pageserver/src/walredo.rs b/pageserver/src/walredo.rs index 9faabfebda..79c2edc96e 100644 --- a/pageserver/src/walredo.rs +++ b/pageserver/src/walredo.rs @@ -21,7 +21,6 @@ use byteorder::{ByteOrder, LittleEndian}; use bytes::{BufMut, Bytes, BytesMut}; use nix::poll::*; -use remote_storage::path_with_suffix_extension; use serde::Serialize; use std::fs; use std::fs::OpenOptions; @@ -36,6 +35,7 @@ use std::sync::Mutex; use std::time::Duration; use std::time::Instant; use tracing::*; +use utils::crashsafe_dir::path_with_suffix_extension; use utils::{bin_ser::BeSer, id::TenantId, lsn::Lsn, nonblock::set_nonblock}; use crate::metrics::{ diff --git a/workspace_hack/Cargo.toml b/workspace_hack/Cargo.toml index 96594bbf96..dc4cbb5284 100644 --- a/workspace_hack/Cargo.toml +++ b/workspace_hack/Cargo.toml @@ -21,13 +21,7 @@ bytes = { version = "1", features = ["serde", "std"] } chrono = { version = "0.4", features = ["clock", "libc", "oldtime", "serde", "std", "time", "winapi"] } either = { version = "1", features = ["use_std"] } fail = { version = "0.5", default-features = false, features = ["failpoints"] } -futures-channel = { version = "0.3", features = ["alloc", "futures-sink", "sink", "std"] } -futures-task = { version = "0.3", default-features = false, features = ["alloc", "std"] } -futures-util = { version = "0.3", default-features = false, features = ["alloc", "async-await", "async-await-macro", "channel", "futures-channel", "futures-io", "futures-macro", "futures-sink", "io", "memchr", "sink", "slab", "std"] } -generic-array = { version = "0.14", default-features = false, features = ["more_lengths"] } hashbrown = { version = "0.12", features = ["ahash", "inline-more", "raw"] } -hex = { version = "0.4", features = ["alloc", "serde", "std"] } -hyper = { version = "0.14", features = ["client", "full", "h2", "http1", "http2", "runtime", "server", "socket2", "stream", "tcp"] } indexmap = { version = "1", default-features = false, features = ["std"] } itoa = { version = "0.4", features = ["i128", "std"] } libc = { version = "0.2", features = ["extra_traits", "std"] }