From 825d3631707016717f05ae5bcb7c112af9feba8f Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki@zenith.tech>
Date: Thu, 24 Mar 2022 12:17:56 +0200
Subject: [PATCH] Remove some unnecessary Ord etc. trait implementations.

It doesn't make much sense to compare TimelineMetadata structs with
< or >. But we depended on that in the remote storage upload code,
so replace BTreeSets with Vecs there.
---
 pageserver/src/layered_repository/metadata.rs |  2 +-
 pageserver/src/remote_storage/storage_sync.rs | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/pageserver/src/layered_repository/metadata.rs b/pageserver/src/layered_repository/metadata.rs
index 960a1b7fe3..99d786c4cd 100644
--- a/pageserver/src/layered_repository/metadata.rs
+++ b/pageserver/src/layered_repository/metadata.rs
@@ -28,7 +28,7 @@ pub const METADATA_FILE_NAME: &str = "metadata";
 /// Metadata stored on disk for each timeline
 ///
 /// The fields correspond to the values we hold in memory, in LayeredTimeline.
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct TimelineMetadata {
     disk_consistent_lsn: Lsn,
     // This is only set if we know it. We track it in memory when the page
diff --git a/pageserver/src/remote_storage/storage_sync.rs b/pageserver/src/remote_storage/storage_sync.rs
index f1483375cb..4ad28e6f8f 100644
--- a/pageserver/src/remote_storage/storage_sync.rs
+++ b/pageserver/src/remote_storage/storage_sync.rs
@@ -142,7 +142,7 @@ lazy_static! {
 /// mpsc approach was picked to allow blocking the sync loop if no tasks are present, to avoid meaningless spinning.
 mod sync_queue {
     use std::{
-        collections::{BTreeSet, HashMap},
+        collections::HashMap,
         sync::atomic::{AtomicUsize, Ordering},
     };
 
@@ -205,9 +205,9 @@ mod sync_queue {
     pub async fn next_task_batch(
         receiver: &mut UnboundedReceiver<SyncTask>,
         mut max_batch_size: usize,
-    ) -> BTreeSet<SyncTask> {
+    ) -> Vec<SyncTask> {
         if max_batch_size == 0 {
-            return BTreeSet::new();
+            return Vec::new();
         }
         let mut tasks = HashMap::with_capacity(max_batch_size);
 
@@ -244,7 +244,7 @@ mod sync_queue {
 
 /// A task to run in the async download/upload loop.
 /// Limited by the number of retries, after certain threshold the failing task gets evicted and the timeline disabled.
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
+#[derive(Debug, Clone)]
 pub struct SyncTask {
     sync_id: ZTenantTimelineId,
     retries: u32,
@@ -261,7 +261,7 @@ impl SyncTask {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
+#[derive(Debug, Clone)]
 enum SyncKind {
     /// A certain amount of images (archive files) to download.
     Download(TimelineDownload),
@@ -281,7 +281,7 @@ impl SyncKind {
 
 /// Local timeline files for upload, appeared after the new checkpoint.
 /// Current checkpoint design assumes new files are added only, no deletions or amendment happens.
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
+#[derive(Debug, Clone)]
 pub struct NewCheckpoint {
     /// Relish file paths in the pageserver workdir, that were added for the corresponding checkpoint.
     layers: Vec<PathBuf>,
@@ -289,7 +289,7 @@ pub struct NewCheckpoint {
 }
 
 /// Info about the remote image files.
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
+#[derive(Debug, Clone)]
 struct TimelineDownload {
     files_to_skip: Arc<BTreeSet<PathBuf>>,
     archives_to_skip: BTreeSet<ArchiveId>,
@@ -485,11 +485,11 @@ async fn loop_step<
     max_sync_errors: NonZeroU32,
 ) -> HashMap<ZTenantId, HashMap<ZTimelineId, TimelineSyncStatusUpdate>> {
     let max_concurrent_sync = max_concurrent_sync.get();
-    let mut next_tasks = BTreeSet::new();
+    let mut next_tasks = Vec::new();
 
     // request the first task in blocking fashion to do less meaningless work
     if let Some(first_task) = sync_queue::next_task(receiver).await {
-        next_tasks.insert(first_task);
+        next_tasks.push(first_task);
     } else {
         debug!("Shutdown requested, stopping");
         return HashMap::new();