[pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options

[pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options

[Nicolas Frey]

this patch series aims to expose configuration of the number of threads
to use for reading and verifying of verify jobs through the datastore's
tuning options.

It expands on the series I sent last week [0], where @Thomas
suggested making this configurable through the tuning options too.
@Chris suggested adding a dedicated worker thread settings to the
datastore instead, which is an approach I don't have a problem with,
but I'd want some more opinions on.

If the docs should be updated to account for this change, I can either
send a follow up or do that in a v2.

Later down the line, it would also be beneficial to add the tape job
threads to these options, though that is outside of the scope of this
series.

[0] https://lore.proxmox.com/pbs-devel/5a731352-1f77-4b15-9a22-7064b70ce519@proxmox.com/T/#t

proxmox:

Nicolas Frey (1):
  pbs-api-types: add verify-job-{read,verify}-threads to tuning options

 pbs-api-types/src/datastore.rs | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)


proxmox-backup:

Nicolas Frey (4):
  datastore: add new thread settings to tuning options
  verify: use tuning options' thread settings for {verify,read}-threads
  ui: tuning: make verify-job-{read,verify}-threads configurable
  ui: verify: fetch default {read,verify} thread values from tuning
    options

 pbs-datastore/src/datastore.rs | 33 +++++++++++++++++++++++++++++
 src/backup/verify.rs           | 14 +++++++++++--
 www/datastore/OptionView.js    | 20 ++++++++++++++++++
 www/window/VerifyAll.js        | 38 ++++++++++++++++++++++++++++++++--
 www/window/VerifyJobEdit.js    | 37 +++++++++++++++++++++++++++++++++
 5 files changed, 138 insertions(+), 4 deletions(-)


Summary over all repositories:
  6 files changed, 152 insertions(+), 5 deletions(-)

-- 
Generated by git-murpp 0.8.1

_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox 1/1] pbs-api-types: add verify-job-{read, verify}-threads to tuning options

[Nicolas Frey]

to determine the default number of threads to use for verify jobs on
the datastore level

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
adds it to the tuning options, though it can easily be moved into its
own setting within the datastore configuration if desired

 pbs-api-types/src/datastore.rs | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index a214ac25..176c0d4a 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -19,7 +19,8 @@ use crate::{
     BACKUP_ID_RE, BACKUP_NS_RE, BACKUP_TIME_RE, BACKUP_TYPE_RE, DATASTORE_NOTIFY_STRING_SCHEMA,
     GC_SCHEDULE_SCHEMA, GROUP_OR_SNAPSHOT_PATH_REGEX_STR, PROXMOX_SAFE_ID_FORMAT,
     PROXMOX_SAFE_ID_REGEX_STR, PRUNE_SCHEDULE_SCHEMA, SHA256_HEX_REGEX, SINGLE_LINE_COMMENT_SCHEMA,
-    SNAPSHOT_PATH_REGEX_STR, UPID,
+    SNAPSHOT_PATH_REGEX_STR, UPID, VERIFY_JOB_READ_THREADS_SCHEMA,
+    VERIFY_JOB_VERIFY_THREADS_SCHEMA,
 };
 
 const_regex! {
@@ -262,6 +263,14 @@ pub const GC_CACHE_CAPACITY_SCHEMA: Schema =
             schema: GC_CACHE_CAPACITY_SCHEMA,
             optional: true,
         },
+        "verify-job-verify-threads": {
+            schema: VERIFY_JOB_VERIFY_THREADS_SCHEMA,
+            optional: true,
+        },
+        "verify-job-read-threads": {
+            schema: VERIFY_JOB_READ_THREADS_SCHEMA,
+            optional: true,
+        },
     },
 )]
 #[derive(Serialize, Deserialize, Default)]
@@ -279,6 +288,10 @@ pub struct DatastoreTuning {
     pub gc_atime_cutoff: Option<usize>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub gc_cache_capacity: Option<usize>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub verify_job_verify_threads: Option<usize>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub verify_job_read_threads: Option<usize>,
 }
 
 pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore tuning options")
-- 
2.47.3


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 1/4] datastore: add new thread settings to tuning options

[Nicolas Frey]

add a new field to the `DataStoreImpl` to store the verify job thread
configuration from the tuning options

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 0a517923..fec9de78 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -146,6 +146,7 @@ pub struct DataStoreImpl {
     sync_level: DatastoreFSyncLevel,
     backend_config: DatastoreBackendConfig,
     lru_store_caching: Option<LocalDatastoreLruCache>,
+    thread_settings: DatastoreThreadSettings,
 }
 
 impl DataStoreImpl {
@@ -162,6 +163,7 @@ impl DataStoreImpl {
             sync_level: Default::default(),
             backend_config: Default::default(),
             lru_store_caching: None,
+            thread_settings: Default::default(),
         })
     }
 }
@@ -256,6 +258,27 @@ impl DatastoreBackend {
     }
 }
 
+#[derive(Clone, Default)]
+/// Amount of threads to use for certain jobs in a datastore.
+pub struct DatastoreThreadSettings {
+    /// # of threads to use to verify in verify job
+    pub verify_job_verify_threads: Option<usize>,
+    /// # of threads to use to read in verify job
+    pub verify_job_read_threads: Option<usize>,
+}
+
+impl DatastoreThreadSettings {
+    pub fn new(
+        verify_job_verify_threads: Option<usize>,
+        verify_job_read_threads: Option<usize>,
+    ) -> Self {
+        Self {
+            verify_job_verify_threads,
+            verify_job_read_threads,
+        }
+    }
+}
+
 impl DataStore {
     // This one just panics on everything
     #[doc(hidden)]
@@ -532,6 +555,11 @@ impl DataStore {
             None
         };
 
+        let thread_settings = DatastoreThreadSettings::new(
+            tuning.verify_job_verify_threads,
+            tuning.verify_job_read_threads,
+        );
+
         Ok(DataStoreImpl {
             chunk_store,
             gc_mutex: Mutex::new(None),
@@ -542,6 +570,7 @@ impl DataStore {
             sync_level: tuning.sync_level.unwrap_or_default(),
             backend_config,
             lru_store_caching,
+            thread_settings,
         })
     }
 
@@ -641,6 +670,10 @@ impl DataStore {
         self.inner.chunk_store.base_path()
     }
 
+    pub fn thread_settings(&self) -> &DatastoreThreadSettings {
+        &self.inner.thread_settings
+    }
+
     /// Returns the absolute path for a backup namespace on this datastore
     pub fn namespace_path(&self, ns: &BackupNamespace) -> PathBuf {
         let mut path = self.base_path();
-- 
2.47.3


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 2/4] verify: use tuning options' thread settings for {verify, read}-threads

[Nicolas Frey]

uses the datastores' verify job thread settings as a secondary
fallback:

read_threads   -> arg? -> datastore.read? -> 1 (default)
verify_threads -> arg? -> datastore.verify? -> 4 (default)

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 src/backup/verify.rs | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 734c7b30..f52d7781 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -73,6 +73,16 @@ impl VerifyWorker {
         verify_threads: Option<usize>,
     ) -> Result<Self, Error> {
         let backend = datastore.backend()?;
+        let thread_settings = datastore.thread_settings();
+
+        let read_threads = read_threads
+            .or(thread_settings.verify_job_read_threads)
+            .unwrap_or(1);
+
+        let verify_threads = verify_threads
+            .or(thread_settings.verify_job_verify_threads)
+            .unwrap_or(4);
+
         Ok(Self {
             worker,
             datastore,
@@ -81,8 +91,8 @@ impl VerifyWorker {
             // start with 64 chunks since we assume there are few corrupt ones
             corrupt_chunks: Arc::new(Mutex::new(HashSet::with_capacity(64))),
             backend,
-            read_threads: read_threads.unwrap_or(1),
-            verify_threads: verify_threads.unwrap_or(4),
+            read_threads,
+            verify_threads,
         })
     }
 
-- 
2.47.3


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 3/4] ui: tuning: make verify-job-{read, verify}-threads configurable

[Nicolas Frey]

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 www/datastore/OptionView.js | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index 2f9aa9eb..138a7b49 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -310,6 +310,26 @@ Ext.define('PBS.Datastore.Options', {
                             deleteEmpty: true,
                             step: 1024,
                         },
+                        {
+                            xtype: 'proxmoxintegerfield',
+                            name: 'verify-job-verify-threads',
+                            fieldLabel: gettext('# of Verify Threads'),
+                            labelWidth: 200,
+                            emptyText: '4',
+                            minValue: 1,
+                            maxValue: 32,
+                            deleteEmpty: true,
+                        },
+                        {
+                            xtype: 'proxmoxintegerfield',
+                            name: 'verify-job-read-threads',
+                            fieldLabel: gettext('# of Read Threads'),
+                            labelWidth: 200,
+                            emptyText: '1',
+                            minValue: 1,
+                            maxValue: 32,
+                            deleteEmpty: true,
+                        },
                     ],
                 },
             },
-- 
2.47.3


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 4/4] ui: verify: fetch default {read, verify} thread values from tuning options

[Nicolas Frey]

making sure that the now default (e.g. emptyText) value matches the
one found at the datastore-tuning level, we fetch the tuning data
and bind it to the respective `emptyText` properties.

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 www/window/VerifyAll.js     | 38 +++++++++++++++++++++++++++++++++++--
 www/window/VerifyJobEdit.js | 37 ++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/www/window/VerifyAll.js b/www/window/VerifyAll.js
index 4239c215..e1670f10 100644
--- a/www/window/VerifyAll.js
+++ b/www/window/VerifyAll.js
@@ -11,6 +11,36 @@ Ext.define('PBS.window.VerifyAll', {
         url: `/admin/datastore/{datastore}/verify`,
     },
 
+    viewModel: {
+        data: {
+            defaultReadThreads: null,
+            defaultVerifyThreads: null,
+        },
+    },
+
+    initComponent: function () {
+        let me = this;
+        me.callParent();
+
+        if (me.datastore === undefined) {
+            return;
+        }
+
+        Proxmox.Utils.API2Request({
+            url: `/api2/extjs/config/datastore/${encodeURIComponent(me.datastore)}`,
+            method: 'GET',
+            success: ({ result }) => {
+                let raw = result?.data?.tuning || {};
+                let tuning = PBS.Utils.parsePropertyString(raw);
+
+                me.getViewModel().set({
+                    defaultReadThreads: tuning['verify-job-read-threads'] ?? 1,
+                    defaultVerifyThreads: tuning['verify-job-verify-threads'] ?? 4,
+                });
+            },
+        });
+    },
+
     submitText: gettext('Verify'),
     isCreate: true,
     showTaskViewer: true,
@@ -84,7 +114,9 @@ Ext.define('PBS.window.VerifyAll', {
                     xtype: 'proxmoxintegerfield',
                     name: 'read-threads',
                     fieldLabel: gettext('# of Read Threads'),
-                    emptyText: '1',
+                    bind: {
+                        emptyText: '{defaultReadThreads}',
+                    },
                     skipEmptyText: true,
                     minValue: 1,
                     maxValue: 32,
@@ -93,7 +125,9 @@ Ext.define('PBS.window.VerifyAll', {
                     xtype: 'proxmoxintegerfield',
                     name: 'verify-threads',
                     fieldLabel: gettext('# of Verify Threads'),
-                    emptyText: '4',
+                    bind: {
+                        emptyText: '{defaultVerifyThreads}',
+                    },
                     skipEmptyText: true,
                     minValue: 1,
                     maxValue: 32,
diff --git a/www/window/VerifyJobEdit.js b/www/window/VerifyJobEdit.js
index 5650ed5c..8edc0865 100644
--- a/www/window/VerifyJobEdit.js
+++ b/www/window/VerifyJobEdit.js
@@ -32,8 +32,38 @@ Ext.define('PBS.window.VerifyJobEdit', {
     viewModel: {
         data: {
             ignoreVerified: true,
+            defaultReadThreads: null,
+            defaultVerifyThreads: null,
         },
     },
+
+    loadThreadDefaults: function (datastore) {
+        Proxmox.Utils.API2Request({
+            url: `/api2/extjs/config/datastore/${encodeURIComponent(datastore)}`,
+            method: 'GET',
+            success: ({ result }) => {
+                let raw = result?.data?.tuning || {};
+                let tuning = PBS.Utils.parsePropertyString(raw);
+
+                this.getViewModel().set({
+                    defaultReadThreads: tuning['verify-job-read-threads'] ?? 1,
+                    defaultVerifyThreads: tuning['verify-job-verify-threads'] ?? 4,
+                });
+            },
+        });
+    },
+
+    initComponent: function () {
+        let me = this;
+        me.callParent();
+
+        if (me.datastore === undefined) {
+            return;
+        }
+
+        me.loadThreadDefaults(me.datastore);
+    },
+
     controller: {
         xclass: 'Ext.app.ViewController',
         control: {
@@ -46,6 +76,7 @@ Ext.define('PBS.window.VerifyJobEdit', {
             let view = this.getView();
             let nsSelector = view.down('pbsNamespaceSelector');
             nsSelector.setDatastore(value);
+            view.loadThreadDefaults(value)
         },
     },
 
@@ -164,6 +195,9 @@ Ext.define('PBS.window.VerifyJobEdit', {
                 cbind: {
                     deleteEmpty: '{!isCreate}',
                 },
+                bind: {
+                    emptyText: '{defaultReadThreads}',
+                },
             },
             {
                 xtype: 'proxmoxintegerfield',
@@ -175,6 +209,9 @@ Ext.define('PBS.window.VerifyJobEdit', {
                 cbind: {
                     deleteEmpty: '{!isCreate}',
                 },
+                bind: {
+                    emptyText: '{defaultVerifyThreads}',
+                },
             },
         ],
         advancedColumn2: [
-- 
2.47.3


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel