[pbs-devel] [PATCH proxmox-backup] datastore: add tuning option for chunk order

[pbs-devel] [PATCH proxmox-backup] datastore: add tuning option for chunk order

[Dominik Csapak]

currently, we sort chunks by inode when verifying or backing up to tape.
we get the inode# by stat'ing each chunk, which may be more expensive
than the gains of reading the chunks in order

Since that is highly dependent on the underlying storage of the datastore,
introduce a tuning option  so that the admin can tune that behaviour
for each datastore.

The default stays the same (sorting by inode)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 39 ++++++++++++++++++++++++++++++++++
 pbs-datastore/src/datastore.rs | 36 ++++++++++++++++++++++++-------
 src/api2/config/datastore.rs   |  5 +++++
 3 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 36279b3a..d0215403 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -167,6 +167,38 @@ pub struct PruneOptions {
     pub keep_yearly: Option<u64>,
 }
 
+#[api]
+#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
+/// The order to sort chunks by
+pub enum ChunkOrder {
+    /// Iterate chunks in the index order
+    None,
+    /// Iterate chunks in inode order
+    Inode,
+}
+
+#[api(
+    properties: {
+        "chunk-order": {
+            type: ChunkOrder,
+            optional: true,
+        },
+    },
+)]
+#[derive(Serialize, Deserialize, Default)]
+#[serde(rename_all = "kebab-case")]
+/// Datastore tuning options
+pub struct DatastoreTuning {
+    /// Iterate chunks in this order
+    pub chunk_order: Option<ChunkOrder>,
+}
+
+pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new(
+    "Datastore tuning options")
+    .format(&ApiStringFormat::PropertyString(&DatastoreTuning::API_SCHEMA))
+    .schema();
+
 #[api(
     properties: {
         name: {
@@ -224,6 +256,10 @@ pub struct PruneOptions {
             optional: true,
             type: bool,
         },
+        tuning: {
+            optional: true,
+            schema: DATASTORE_TUNING_STRING_SCHEMA,
+        },
     }
 )]
 #[derive(Serialize,Deserialize,Updater)]
@@ -261,6 +297,9 @@ pub struct DataStoreConfig {
     /// Send notification only for job errors
     #[serde(skip_serializing_if="Option::is_none")]
     pub notify: Option<String>,
+    /// Datastore tuning options
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub tuning: Option<String>,
 }
 
 #[api(
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 7044e074..8397da00 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -9,13 +9,18 @@ use std::time::Duration;
 use anyhow::{bail, format_err, Error};
 use lazy_static::lazy_static;
 
+use proxmox_schema::ApiType;
+
 use proxmox_sys::fs::{replace_file, file_read_optional_string, CreateOptions};
 use proxmox_sys::process_locker::ProcessLockSharedGuard;
 use proxmox_sys::WorkerTaskContext;
 use proxmox_sys::{task_log, task_warn};
 use proxmox_sys::fs::{lock_dir_noblock, DirLockGuard};
 
-use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus, HumanByte};
+use pbs_api_types::{
+    UPID, DataStoreConfig, Authid, GarbageCollectionStatus, HumanByte,
+    ChunkOrder, DatastoreTuning,
+};
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
 use crate::DataBlob;
@@ -57,12 +62,11 @@ pub struct DataStore {
     gc_mutex: Mutex<()>,
     last_gc_status: Mutex<GarbageCollectionStatus>,
     verify_new: bool,
+    chunk_order: ChunkOrder,
 }
 
 impl DataStore {
-
     pub fn lookup_datastore(name: &str) -> Result<Arc<DataStore>, Error> {
-
         let (config, _digest) = pbs_config::datastore::config()?;
         let config: DataStoreConfig = config.lookup("datastore", name)?;
         let path = PathBuf::from(&config.path);
@@ -116,11 +120,17 @@ impl DataStore {
             GarbageCollectionStatus::default()
         };
 
+        let tuning: DatastoreTuning = serde_json::from_value(
+            DatastoreTuning::API_SCHEMA.parse_property_string(config.tuning.as_deref().unwrap_or(""))?
+        )?;
+        let chunk_order = tuning.chunk_order.unwrap_or(ChunkOrder::Inode);
+
         Ok(Self {
             chunk_store: Arc::new(chunk_store),
             gc_mutex: Mutex::new(()),
             last_gc_status: Mutex::new(gc_status),
             verify_new: config.verify_new.unwrap_or(false),
+            chunk_order,
         })
     }
 
@@ -907,16 +917,26 @@ impl DataStore {
                 continue;
             }
 
-            let ino = match self.stat_chunk(&info.digest) {
-                Err(_) => u64::MAX, // could not stat, move to end of list
-                Ok(metadata) => metadata.ino(),
+            let ino = match self.chunk_order {
+                ChunkOrder::Inode => {
+                    match self.stat_chunk(&info.digest) {
+                        Err(_) => u64::MAX, // could not stat, move to end of list
+                        Ok(metadata) => metadata.ino(),
+                    }
+                }
+                ChunkOrder::None => 0,
             };
 
             chunk_list.push((pos, ino));
         }
 
-        // sorting by inode improves data locality, which makes it lots faster on spinners
-        chunk_list.sort_unstable_by(|(_, ino_a), (_, ino_b)| ino_a.cmp(ino_b));
+        match self.chunk_order {
+            // sorting by inode improves data locality, which makes it lots faster on spinners
+            ChunkOrder::Inode => {
+                chunk_list.sort_unstable_by(|(_, ino_a), (_, ino_b)| ino_a.cmp(ino_b))
+            }
+            ChunkOrder::None => {}
+        }
 
         Ok(chunk_list)
     }
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 60bc3c0e..f8f98afe 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -184,6 +184,8 @@ pub enum DeletableProperty {
     notify_user,
     /// Delete the notify property
     notify,
+    /// Delete the tuning property
+    tuning,
 }
 
 #[api(
@@ -250,6 +252,7 @@ pub fn update_datastore(
                 DeletableProperty::verify_new => { data.verify_new = None; },
                 DeletableProperty::notify => { data.notify = None; },
                 DeletableProperty::notify_user => { data.notify_user = None; },
+                DeletableProperty::tuning => { data.tuning = None; },
             }
         }
     }
@@ -295,6 +298,8 @@ pub fn update_datastore(
 
     if update.notify_user.is_some() { data.notify_user = update.notify_user; }
 
+    if update.tuning.is_some() { data.tuning = update.tuning; }
+
     config.set_data(&name, "datastore", &data)?;
 
     pbs_config::datastore::save_config(&config)?;
-- 
2.30.2

[pbs-devel] applied: [PATCH proxmox-backup] datastore: add tuning option for chunk order

[Dietmar Maurer]

applied.