Move path manipulation function to utils

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",

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"

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<Path>, 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<RemoteStorageConfig> {
         let local_path = toml.get("local_path");
@@ -448,35 +430,6 @@ fn parse_toml_string(name: &str, item: &Item) -> anyhow::Result<String> {
 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());

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};
 

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<Path>) -> 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<Path>, 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"
+        );
+    }
 }

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},

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.

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 {

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,

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::{

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"] }