From 9a69b6cb941cb58a053b5f96f9150d8976e5dc5d Mon Sep 17 00:00:00 2001 From: Joonas Koivunen Date: Fri, 14 Jul 2023 18:59:16 +0300 Subject: [PATCH] Demote deletion warning, list files (#4688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handle test failures like: ``` AssertionError: assert not ['$ts WARN delete_timeline{tenant_id=X timeline_id=Y}: About to remove 1 files\n'] ``` Instead of logging: ``` WARN delete_timeline{tenant_id=X timeline_id=Y}: Found 1 files not bound to index_file.json, proceeding with their deletion WARN delete_timeline{tenant_id=X timeline_id=Y}: About to remove 1 files ``` For each one operation of timeline deletion, list all unref files with `info!`, and then continue to delete them with the added spice of logging the rare/never happening non-utf8 name with `warn!`. Rationale for `info!` instead of `warn!`: this is a normal operation; like we had mentioned in `test_import.py` -- basically whenever we delete a timeline which is not idle. Rationale for N * (`ìnfo!`|`warn!`): symmetry for the layer deletions; if we could ever need those, we could also need these for layer files which are not yet mentioned in `index_part.json`. --------- Co-authored-by: Christian Schwarz --- libs/remote_storage/src/lib.rs | 6 ++++++ pageserver/src/tenant/remote_timeline_client.rs | 14 ++++++++------ test_runner/regress/test_import.py | 6 ------ test_runner/regress/test_remote_storage.py | 3 --- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/libs/remote_storage/src/lib.rs b/libs/remote_storage/src/lib.rs index 0e9c237e1e..90f9efa146 100644 --- a/libs/remote_storage/src/lib.rs +++ b/libs/remote_storage/src/lib.rs @@ -50,6 +50,12 @@ const REMOTE_STORAGE_PREFIX_SEPARATOR: char = '/'; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct RemotePath(PathBuf); +impl std::fmt::Display for RemotePath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0.display()) + } +} + impl RemotePath { pub fn new(relative_path: &Path) -> anyhow::Result { anyhow::ensure!( diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index 8c04794e92..0f5a585f3f 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -840,14 +840,16 @@ impl RemoteTimelineClient { let remaining: Vec = remaining .into_iter() .filter(|p| p.object_name() != Some(IndexPart::FILE_NAME)) + .inspect(|path| { + if let Some(name) = path.object_name() { + info!(%name, "deleting a file not referenced from index_part.json"); + } else { + warn!(%path, "deleting a nameless or non-utf8 object not referenced from index_part.json"); + } + }) .collect(); if !remaining.is_empty() { - warn!( - "Found {} files not bound to index_file.json, proceeding with their deletion", - remaining.len() - ); - warn!("About to remove {} files", remaining.len()); self.storage_impl.delete_objects(&remaining).await?; } @@ -856,7 +858,7 @@ impl RemoteTimelineClient { debug!("deleting index part"); self.storage_impl.delete(&index_file_path).await?; - info!(deletions_queued, "done deleting, including index_part.json"); + info!(prefix=%timeline_storage_path, referenced=deletions_queued, not_referenced=%remaining.len(), "done deleting in timeline prefix, including index_part.json"); Ok(()) } diff --git a/test_runner/regress/test_import.py b/test_runner/regress/test_import.py index 141c69b230..9248c42555 100644 --- a/test_runner/regress/test_import.py +++ b/test_runner/regress/test_import.py @@ -149,12 +149,6 @@ def test_import_from_vanilla(test_output_dir, pg_bin, vanilla_pg, neon_env_build ".*WARN.*ignored .* unexpected bytes after the tar archive.*" ) - # NOTE: delete can easily come before upload operations are completed - # https://github.com/neondatabase/neon/issues/4326 - env.pageserver.allowed_errors.append( - ".*files not bound to index_file.json, proceeding with their deletion.*" - ) - timeline_delete_wait_completed(client, tenant, timeline) # Importing correct backup works diff --git a/test_runner/regress/test_remote_storage.py b/test_runner/regress/test_remote_storage.py index 1b6e39ff31..13bc01f609 100644 --- a/test_runner/regress/test_remote_storage.py +++ b/test_runner/regress/test_remote_storage.py @@ -598,9 +598,6 @@ def test_timeline_deletion_with_files_stuck_in_upload_queue( ".* ERROR .*Error processing HTTP request: InternalServerError\\(timeline is Stopping" ) - env.pageserver.allowed_errors.append( - ".*files not bound to index_file.json, proceeding with their deletion.*" - ) timeline_delete_wait_completed(client, tenant_id, timeline_id) assert not timeline_path.exists()