rust/neon
1
0
Fork 0
mirror of https://github.com/neondatabase/neon.git synced 2026-06-04 22:10:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simplify None handling for Generation in LayerfileMetadata

Browse Source
This commit is contained in:
John Spray
2023-08-29 16:43:47 +01:00
parent 980d3ba8b0
commit 4a0e2d1290
5 changed files with 32 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
pageserver/src/tenant/remote_timeline_client.rs
View File

@@ -664,16 +664,15 @@ impl RemoteTimelineClient {
// Remove from latest_files, learning the file's remote generation in the process
let meta = upload_queue.latest_files.remove(name);
if meta.is_none() {
if let Some(meta) = meta {
upload_queue.latest_files_changes_since_metadata_upload_scheduled += 1;
Some((name, meta.generation))
} else {
// This is unexpected: latest_files is meant to be kept up to
// date. We can't delete the layer if we have forgotten what
// generation it was in.
warn!("Deleting layer {name} not found in latest_files list");
None
} else {
upload_queue.latest_files_changes_since_metadata_upload_scheduled += 1;
let generation = meta.and_then(|m| m.generation);
Some((name, generation))
}
})
.collect();
@@ -1401,20 +1400,12 @@ pub fn remote_layer_path(
layer_file_name: &LayerFileName,
layer_meta: &LayerFileMetadata,
) -> RemotePath {
let path = if let Some(generation) = layer_meta.generation {
// Generation-aware key format
format!(
"tenants/{tenant_id}/{TIMELINES_SEGMENT_NAME}/{timeline_id}/{0}{1}",
layer_file_name.file_name(),
generation.get_suffix()
)
} else {
// Pre-generation key format
format!(
"tenants/{tenant_id}/{TIMELINES_SEGMENT_NAME}/{timeline_id}/{0}",
layer_file_name.file_name(),
)
};
// Generation-aware key format
let path = format!(
"tenants/{tenant_id}/{TIMELINES_SEGMENT_NAME}/{timeline_id}/{0}{1}",
layer_file_name.file_name(),
layer_meta.generation.get_suffix()
);
RemotePath::from_string(&path).expect("Failed to construct path")
}

4
pageserver/src/tenant/remote_timeline_client/delete.rs
View File

@@ -14,14 +14,14 @@ pub(super) async fn delete_layer<'a>(
conf: &'static PageServerConf,
storage: &'a GenericRemoteStorage,
local_layer_path: &'a Path,
generation: Option<Generation>,
generation: Generation,
) -> anyhow::Result<()> {
fail::fail_point!("before-delete-layer", |_| {
anyhow::bail!("failpoint before-delete-layer")
});
debug!("Deleting layer from remote storage: {local_layer_path:?}",);
let path_to_delete = remote_path(conf, local_layer_path, generation)?;
let path_to_delete = remote_path(conf, local_layer_path, Some(generation))?;
// We don't want to print an error if the delete failed if the file has
// already been deleted. Thankfully, in this situation S3 already

31
pageserver/src/tenant/remote_timeline_client/index.rs
View File

@@ -21,12 +21,11 @@ use utils::lsn::Lsn;
/// Fields have to be `Option`s because remote [`IndexPart`]'s can be from different version, which
/// might have less or more metadata depending if upgrading or rolling back an upgrade.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
#[cfg_attr(test, derive(Default))]
//#[cfg_attr(test, derive(Default))]
pub struct LayerFileMetadata {
file_size: u64,
// Optional for backward compatibility: older data will not have a generation set
pub(crate) generation: Option<Generation>,
pub(crate) generation: Generation,
}
impl From<&'_ IndexLayerMetadata> for LayerFileMetadata {
@@ -42,7 +41,7 @@ impl LayerFileMetadata {
pub fn new(file_size: u64, generation: Generation) -> Self {
LayerFileMetadata {
file_size,
generation: Some(generation),
generation,
}
}
@@ -142,18 +141,14 @@ impl TryFrom<&UploadQueueInitialized> for IndexPart {
}
}
fn generation_is_none(g: &Option<Generation>) -> bool {
g.map(|g| g.is_none()).unwrap_or(true)
}
/// Serialized form of [`LayerFileMetadata`].
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Default)]
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
pub struct IndexLayerMetadata {
pub(super) file_size: u64,
#[serde(default)]
#[serde(skip_serializing_if = "generation_is_none")]
pub(super) generation: Option<Generation>,
#[serde(default = "Generation::none")]
#[serde(skip_serializing_if = "Generation::is_none")]
pub(super) generation: Generation,
}
impl From<&'_ LayerFileMetadata> for IndexLayerMetadata {
@@ -189,13 +184,13 @@ mod tests {
layer_metadata: HashMap::from([
("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9".parse().unwrap(), IndexLayerMetadata {
file_size: 25600000,
generation: None,
generation: Generation::none()
}),
("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D8-00000000016B5A51".parse().unwrap(), IndexLayerMetadata {
// serde_json should always parse this but this might be a double with jq for
// example.
file_size: 9007199254741001,
generation: None,
generation: Generation::none()
})
]),
disk_consistent_lsn: "0/16960E8".parse::<Lsn>().unwrap(),
@@ -228,13 +223,13 @@ mod tests {
layer_metadata: HashMap::from([
("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9".parse().unwrap(), IndexLayerMetadata {
file_size: 25600000,
generation: None
generation: Generation::none()
}),
("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D8-00000000016B5A51".parse().unwrap(), IndexLayerMetadata {
// serde_json should always parse this but this might be a double with jq for
// example.
file_size: 9007199254741001,
generation: None
generation: Generation::none()
})
]),
disk_consistent_lsn: "0/16960E8".parse::<Lsn>().unwrap(),
@@ -268,13 +263,13 @@ mod tests {
layer_metadata: HashMap::from([
("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__0000000001696070-00000000016960E9".parse().unwrap(), IndexLayerMetadata {
file_size: 25600000,
generation: None
generation: Generation::none()
}),
("000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__00000000016B59D8-00000000016B5A51".parse().unwrap(), IndexLayerMetadata {
// serde_json should always parse this but this might be a double with jq for
// example.
file_size: 9007199254741001,
generation: None
generation: Generation::none()
})
]),
disk_consistent_lsn: "0/16960E8".parse::<Lsn>().unwrap(),

6
pageserver/src/tenant/timeline/init.rs
View File

@@ -147,7 +147,11 @@ pub(super) fn reconcile(
Err(FutureLayer { local })
} else {
Ok(match (local, remote) {
(Some(local), Some(remote)) if local != remote => UseRemote { local, remote },
(Some(local), Some(remote)) if local != remote => {
assert_eq!(local.generation, remote.generation);
UseRemote { local, remote }
}
(Some(x), Some(_)) => UseLocal(x),
(None, Some(x)) => Evicted(x),
(Some(x), None) => NeedsUpload(x),

4
pageserver/src/tenant/upload_queue.rs
View File

@@ -206,7 +206,7 @@ pub(crate) struct Delete {
pub(crate) file_kind: RemoteOpFileKind,
pub(crate) layer_file_name: LayerFileName,
pub(crate) scheduled_from_timeline_delete: bool,
pub(crate) generation: Option<Generation>,
pub(crate) generation: Generation,
}
#[derive(Debug)]
@@ -243,7 +243,7 @@ impl std::fmt::Display for UploadOp {
"Delete(path: {}, scheduled_from_timeline_delete: {}, gen: {})",
delete.layer_file_name.file_name(),
delete.scheduled_from_timeline_delete,
delete.generation.unwrap_or(Generation::none())
delete.generation
),
UploadOp::Barrier(_) => write!(f, "Barrier"),
}