From c0480facc104cf6fc699d27dc3fd6cde47e1df89 Mon Sep 17 00:00:00 2001
From: Kirill Bulatov <kirill@neon.tech>
Date: Mon, 5 Dec 2022 11:41:12 +0100
Subject: [PATCH] Rename RelativePath to RemotePath

Improve rustdocs a bit
---
 pageserver/src/storage_sync2.rs          | 12 +++----
 pageserver/src/storage_sync2/download.rs |  4 +--
 pageserver/src/storage_sync2/index.rs    | 46 ++++++++++++------------
 pageserver/src/tenant/timeline.rs        | 10 +++---
 4 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/pageserver/src/storage_sync2.rs b/pageserver/src/storage_sync2.rs
index 44b82a6a8c..5b3225028f 100644
--- a/pageserver/src/storage_sync2.rs
+++ b/pageserver/src/storage_sync2.rs
@@ -217,7 +217,7 @@ use crate::metrics::RemoteOpKind;
 use crate::metrics::REMOTE_UPLOAD_QUEUE_UNFINISHED_TASKS;
 use crate::{
     config::PageServerConf,
-    storage_sync::index::{LayerFileMetadata, RelativePath},
+    storage_sync::index::{LayerFileMetadata, RemotePath},
     task_mgr,
     task_mgr::TaskKind,
     task_mgr::BACKGROUND_RUNTIME,
@@ -287,7 +287,7 @@ struct UploadQueueInitialized {
 
     /// All layer files stored in the remote storage, taking into account all
     /// in-progress and queued operations
-    latest_files: HashMap<RelativePath, LayerFileMetadata>,
+    latest_files: HashMap<RemotePath, LayerFileMetadata>,
 
     /// Metadata stored in the remote storage, taking into account all
     /// in-progress and queued operations.
@@ -510,7 +510,7 @@ impl RemoteTimelineClient {
     /// On success, returns the size of the downloaded file.
     pub async fn download_layer_file(
         &self,
-        path: &RelativePath,
+        path: &RemotePath,
         layer_metadata: &LayerFileMetadata,
     ) -> anyhow::Result<u64> {
         let downloaded_size = download::download_layer_file(
@@ -612,7 +612,7 @@ impl RemoteTimelineClient {
             "file size not initialized in metadata"
         );
 
-        let relative_path = RelativePath::strip_base_path(
+        let relative_path = RemotePath::strip_base_path(
             &self.conf.timeline_path(&self.timeline_id, &self.tenant_id),
             path,
         )?;
@@ -644,7 +644,7 @@ impl RemoteTimelineClient {
         // Convert the paths into RelativePaths, and gather other information we need.
         let mut relative_paths = Vec::with_capacity(paths.len());
         for path in paths {
-            relative_paths.push(RelativePath::strip_base_path(
+            relative_paths.push(RemotePath::strip_base_path(
                 &self.conf.timeline_path(&self.timeline_id, &self.tenant_id),
                 path,
             )?);
@@ -1093,7 +1093,7 @@ mod tests {
         TimelineMetadata::from_bytes(&metadata.to_bytes().unwrap()).unwrap()
     }
 
-    fn assert_file_list(a: &HashSet<RelativePath>, b: &[&str]) {
+    fn assert_file_list(a: &HashSet<RemotePath>, b: &[&str]) {
         let xx = PathBuf::from("");
         let mut avec: Vec<String> = a
             .iter()
diff --git a/pageserver/src/storage_sync2/download.rs b/pageserver/src/storage_sync2/download.rs
index 12b858fb57..d68455ea2b 100644
--- a/pageserver/src/storage_sync2/download.rs
+++ b/pageserver/src/storage_sync2/download.rs
@@ -15,7 +15,7 @@ use utils::crashsafe::path_with_suffix_extension;
 use utils::id::{TenantId, TimelineId};
 
 use super::index::IndexPart;
-use super::RelativePath;
+use super::RemotePath;
 
 async fn fsync_path(path: impl AsRef<std::path::Path>) -> Result<(), std::io::Error> {
     fs::File::open(path).await?.sync_all().await
@@ -31,7 +31,7 @@ pub async fn download_layer_file<'a>(
     storage: &'a GenericRemoteStorage,
     tenant_id: TenantId,
     timeline_id: TimelineId,
-    path: &'a RelativePath,
+    path: &'a RemotePath,
     layer_metadata: &'a LayerFileMetadata,
 ) -> anyhow::Result<u64> {
     let timeline_path = conf.timeline_path(&timeline_id, &tenant_id);
diff --git a/pageserver/src/storage_sync2/index.rs b/pageserver/src/storage_sync2/index.rs
index 99dd2eae6a..a1da37b826 100644
--- a/pageserver/src/storage_sync2/index.rs
+++ b/pageserver/src/storage_sync2/index.rs
@@ -15,12 +15,14 @@ use crate::tenant::metadata::TimelineMetadata;
 
 use utils::lsn::Lsn;
 
-/// A part of the filesystem path, that needs a root to become a path again.
+/// Path on the remote storage, relative to some inner prefix.
+/// The prefix is an implementation detail, that allows representing local paths
+/// as the remote ones, stripping the local storage prefix away.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
 #[serde(transparent)]
-pub struct RelativePath(PathBuf);
+pub struct RemotePath(PathBuf);
 
-impl RelativePath {
+impl RemotePath {
     pub fn new(relative_path: &Path) -> Self {
         debug_assert!(
             relative_path.is_relative(),
@@ -96,22 +98,22 @@ pub struct IndexPart {
     #[serde(default)]
     version: usize,
 
-    /// Each of the layers present on remote storage.
+    /// Layer names, which are stored on the remote storage.
     ///
     /// Additional metadata can might exist in `layer_metadata`.
-    pub timeline_layers: HashSet<RelativePath>,
+    pub timeline_layers: HashSet<RemotePath>,
 
     /// FIXME: unused field. This should be removed, but that changes the on-disk format,
     /// so we need to make sure we're backwards- (and maybe forwards-) compatible
     /// First pass is to move it to Optional and the next would be its removal
-    missing_layers: Option<HashSet<RelativePath>>,
+    missing_layers: Option<HashSet<RemotePath>>,
 
-    /// Per layer file metadata, which can be present for a present or missing layer file.
+    /// Per layer file name metadata, which can be present for a present or missing layer file.
     ///
     /// Older versions of `IndexPart` will not have this property or have only a part of metadata
     /// that latest version stores.
     #[serde(default)]
-    pub layer_metadata: HashMap<RelativePath, IndexLayerMetadata>,
+    pub layer_metadata: HashMap<RemotePath, IndexLayerMetadata>,
 
     // 'disk_consistent_lsn' is a copy of the 'disk_consistent_lsn' in the metadata.
     // It's duplicated here for convenience.
@@ -129,7 +131,7 @@ impl IndexPart {
     pub const FILE_NAME: &'static str = "index_part.json";
 
     pub fn new(
-        layers_and_metadata: HashMap<RelativePath, LayerFileMetadata>,
+        layers_and_metadata: HashMap<RemotePath, LayerFileMetadata>,
         disk_consistent_lsn: Lsn,
         metadata_bytes: Vec<u8>,
     ) -> Self {
@@ -172,9 +174,9 @@ impl From<&'_ LayerFileMetadata> for IndexLayerMetadata {
 }
 
 fn separate_paths_and_metadata(
-    input: &HashMap<RelativePath, LayerFileMetadata>,
-    output: &mut HashSet<RelativePath>,
-    layer_metadata: &mut HashMap<RelativePath, IndexLayerMetadata>,
+    input: &HashMap<RemotePath, LayerFileMetadata>,
+    output: &mut HashSet<RemotePath>,
+    layer_metadata: &mut HashMap<RemotePath, IndexLayerMetadata>,
 ) {
     for (path, metadata) in input {
         let metadata = IndexLayerMetadata::from(metadata);
@@ -198,8 +200,8 @@ mod tests {
 
         let expected = IndexPart {
             version: 0,
-            timeline_layers: HashSet::from([RelativePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9"))]),
-            missing_layers: Some(HashSet::from([RelativePath(PathBuf::from("not_a_real_layer_but_adding_coverage"))])),
+            timeline_layers: HashSet::from([RemotePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9"))]),
+            missing_layers: Some(HashSet::from([RemotePath(PathBuf::from("not_a_real_layer_but_adding_coverage"))])),
             layer_metadata: HashMap::default(),
             disk_consistent_lsn: "0/16960E8".parse::<Lsn>().unwrap(),
             metadata_bytes: [113,11,159,210,0,54,0,4,0,0,0,0,1,105,96,232,1,0,0,0,0,1,105,96,112,0,0,0,0,0,0,0,0,0,0,0,0,0,1,105,96,112,0,0,0,0,1,105,96,112,0,0,0,14,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0].to_vec(),
@@ -226,13 +228,13 @@ mod tests {
         let expected = IndexPart {
             // note this is not verified, could be anything, but exists for humans debugging.. could be the git version instead?
             version: 1,
-            timeline_layers: HashSet::from([RelativePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9"))]),
-            missing_layers: Some(HashSet::from([RelativePath(PathBuf::from("not_a_real_layer_but_adding_coverage"))])),
+            timeline_layers: HashSet::from([RemotePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9"))]),
+            missing_layers: Some(HashSet::from([RemotePath(PathBuf::from("not_a_real_layer_but_adding_coverage"))])),
             layer_metadata: HashMap::from([
-                (RelativePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9")), IndexLayerMetadata {
+                (RemotePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9")), IndexLayerMetadata {
                     file_size: Some(25600000),
                 }),
-                (RelativePath(PathBuf::from("not_a_real_layer_but_adding_coverage")), IndexLayerMetadata {
+                (RemotePath(PathBuf::from("not_a_real_layer_but_adding_coverage")), IndexLayerMetadata {
                     // serde_json should always parse this but this might be a double with jq for
                     // example.
                     file_size: Some(9007199254741001),
@@ -262,12 +264,12 @@ mod tests {
         let expected = IndexPart {
             // note this is not verified, could be anything, but exists for humans debugging.. could be the git version instead?
             version: 1,
-            timeline_layers: [RelativePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9"))].into_iter().collect(),
+            timeline_layers: [RemotePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9"))].into_iter().collect(),
             layer_metadata: HashMap::from([
-                (RelativePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9")), IndexLayerMetadata {
+                (RemotePath(PathBuf::from("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9")), IndexLayerMetadata {
                     file_size: Some(25600000),
                 }),
-                (RelativePath(PathBuf::from("not_a_real_layer_but_adding_coverage")), IndexLayerMetadata {
+                (RemotePath(PathBuf::from("not_a_real_layer_but_adding_coverage")), IndexLayerMetadata {
                     // serde_json should always parse this but this might be a double with jq for
                     // example.
                     file_size: Some(9007199254741001),
@@ -275,7 +277,7 @@ mod tests {
             ]),
             disk_consistent_lsn: "0/16960E8".parse::<Lsn>().unwrap(),
             metadata_bytes: [112,11,159,210,0,54,0,4,0,0,0,0,1,105,96,232,1,0,0,0,0,1,105,96,112,0,0,0,0,0,0,0,0,0,0,0,0,0,1,105,96,112,0,0,0,0,1,105,96,112,0,0,0,14,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0].to_vec(),
-            missing_layers: None::<HashSet<RelativePath>>,
+            missing_layers: None::<HashSet<RemotePath>>,
         };
 
         let part = serde_json::from_str::<IndexPart>(example).unwrap();
diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs
index e42e887524..1bf967c4bf 100644
--- a/pageserver/src/tenant/timeline.rs
+++ b/pageserver/src/tenant/timeline.rs
@@ -19,7 +19,7 @@ use std::sync::atomic::{AtomicBool, AtomicI64, Ordering as AtomicOrdering};
 use std::sync::{Arc, Mutex, MutexGuard, RwLock};
 use std::time::{Duration, Instant, SystemTime};
 
-use crate::storage_sync::index::{IndexPart, RelativePath};
+use crate::storage_sync::index::{IndexPart, RemotePath};
 use crate::storage_sync::RemoteTimelineClient;
 use crate::tenant::{
     delta_layer::{DeltaLayer, DeltaLayerWriter},
@@ -1013,7 +1013,7 @@ impl Timeline {
         local_filenames.retain(|path| {
             let layer_metadata = index_part
                 .layer_metadata
-                .get(&RelativePath::new(path))
+                .get(&RemotePath::new(path))
                 .map(LayerFileMetadata::from)
                 .unwrap_or(LayerFileMetadata::MISSING);
 
@@ -1062,7 +1062,7 @@ impl Timeline {
 
             let layer_metadata = index_part
                 .layer_metadata
-                .get(&RelativePath::new(path))
+                .get(&RemotePath::new(path))
                 .map(LayerFileMetadata::from)
                 .unwrap_or(LayerFileMetadata::MISSING);
 
@@ -1077,7 +1077,7 @@ impl Timeline {
 
                 trace!("downloading image file: {}", path.display());
                 let sz = remote_client
-                    .download_layer_file(&RelativePath::new(path), &layer_metadata)
+                    .download_layer_file(&RemotePath::new(path), &layer_metadata)
                     .await
                     .context("download image layer")?;
                 trace!("done");
@@ -1107,7 +1107,7 @@ impl Timeline {
 
                 trace!("downloading delta file: {}", path.display());
                 let sz = remote_client
-                    .download_layer_file(&RelativePath::new(path), &layer_metadata)
+                    .download_layer_file(&RemotePath::new(path), &layer_metadata)
                     .await
                     .context("download delta layer")?;
                 trace!("done");