public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options
@ 2025-11-21 12:32 Nicolas Frey
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox 1/1] pbs-api-types: add verify-job-{read, verify}-threads to tuning options Nicolas Frey
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Nicolas Frey @ 2025-11-21 12:32 UTC (permalink / raw)
  To: pbs-devel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH proxmox 1/1] pbs-api-types: add verify-job-{read, verify}-threads to tuning options
  2025-11-21 12:32 [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Nicolas Frey
@ 2025-11-21 12:32 ` Nicolas Frey
  2025-11-21 14:15   ` Christian Ebner
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 1/4] datastore: add new thread settings " Nicolas Frey
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Nicolas Frey @ 2025-11-21 12:32 UTC (permalink / raw)
  To: pbs-devel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 1/4] datastore: add new thread settings to tuning options
  2025-11-21 12:32 [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Nicolas Frey
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox 1/1] pbs-api-types: add verify-job-{read, verify}-threads to tuning options Nicolas Frey
@ 2025-11-21 12:32 ` Nicolas Frey
  2025-11-21 14:15   ` Christian Ebner
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 2/4] verify: use tuning options' thread settings for {verify, read}-threads Nicolas Frey
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Nicolas Frey @ 2025-11-21 12:32 UTC (permalink / raw)
  To: pbs-devel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 2/4] verify: use tuning options' thread settings for {verify, read}-threads
  2025-11-21 12:32 [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Nicolas Frey
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox 1/1] pbs-api-types: add verify-job-{read, verify}-threads to tuning options Nicolas Frey
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 1/4] datastore: add new thread settings " Nicolas Frey
@ 2025-11-21 12:32 ` Nicolas Frey
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 3/4] ui: tuning: make verify-job-{read, verify}-threads configurable Nicolas Frey
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Nicolas Frey @ 2025-11-21 12:32 UTC (permalink / raw)
  To: pbs-devel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 3/4] ui: tuning: make verify-job-{read, verify}-threads configurable
  2025-11-21 12:32 [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Nicolas Frey
                   ` (2 preceding siblings ...)
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 2/4] verify: use tuning options' thread settings for {verify, read}-threads Nicolas Frey
@ 2025-11-21 12:32 ` Nicolas Frey
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 4/4] ui: verify: fetch default {read, verify} thread values from tuning options Nicolas Frey
  2025-11-21 14:15 ` [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Christian Ebner
  5 siblings, 0 replies; 9+ messages in thread
From: Nicolas Frey @ 2025-11-21 12:32 UTC (permalink / raw)
  To: pbs-devel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 4/4] ui: verify: fetch default {read, verify} thread values from tuning options
  2025-11-21 12:32 [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Nicolas Frey
                   ` (3 preceding siblings ...)
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 3/4] ui: tuning: make verify-job-{read, verify}-threads configurable Nicolas Frey
@ 2025-11-21 12:32 ` Nicolas Frey
  2025-11-21 14:15 ` [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Christian Ebner
  5 siblings, 0 replies; 9+ messages in thread
From: Nicolas Frey @ 2025-11-21 12:32 UTC (permalink / raw)
  To: pbs-devel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pbs-devel] [PATCH proxmox 1/1] pbs-api-types: add verify-job-{read, verify}-threads to tuning options
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox 1/1] pbs-api-types: add verify-job-{read, verify}-threads to tuning options Nicolas Frey
@ 2025-11-21 14:15   ` Christian Ebner
  0 siblings, 0 replies; 9+ messages in thread
From: Christian Ebner @ 2025-11-21 14:15 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Nicolas Frey

On 11/21/25 1:32 PM, Nicolas Frey wrote:
> 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": {

nit: these are user facing and a bit clumsy, especially if to be set via 
cli. so maybe `default-verificaton-workers`?

> +            schema: VERIFY_JOB_VERIFY_THREADS_SCHEMA,
> +            optional: true,
> +        },
> +        "verify-job-read-threads": {

... same: maybe `default-verification-readers`. But no hard opinion 
here, as at least your suggestions make the correlation to the now 
pre-existing per verify job configuration parameters clearer.

> +            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")



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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options
  2025-11-21 12:32 [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Nicolas Frey
                   ` (4 preceding siblings ...)
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 4/4] ui: verify: fetch default {read, verify} thread values from tuning options Nicolas Frey
@ 2025-11-21 14:15 ` Christian Ebner
  5 siblings, 0 replies; 9+ messages in thread
From: Christian Ebner @ 2025-11-21 14:15 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Nicolas Frey

On 11/21/25 1:32 PM, Nicolas Frey wrote:
> 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

Nice work!

Gave this series a quick spin an tested:
- that the settings are applied
- edit fields show and update the configured default
- settings are deleted if removed from the config
- verification tasks pick up the configured values
- cli commands for datastore config and verify work as expected

What is still missing is adapting the render_tuning_options in 
www/Utils.js to cover these new parameters as well and adaption to the docs.

Other than that with and a few nit inline to be addressed:

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>



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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup 1/4] datastore: add new thread settings to tuning options
  2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 1/4] datastore: add new thread settings " Nicolas Frey
@ 2025-11-21 14:15   ` Christian Ebner
  0 siblings, 0 replies; 9+ messages in thread
From: Christian Ebner @ 2025-11-21 14:15 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Nicolas Frey

On 11/21/25 1:31 PM, Nicolas Frey wrote:
> 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(

nit: no need to make the constructor pub, this can be limited to the 
module scope.

> +        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
> +    }
> +

nit: please add a short docstring for this method, although there are 
pre-existing methods without, we should add these moving forward

>       /// 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();



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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-21 14:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-21 12:32 [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Nicolas Frey
2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox 1/1] pbs-api-types: add verify-job-{read, verify}-threads to tuning options Nicolas Frey
2025-11-21 14:15   ` Christian Ebner
2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 1/4] datastore: add new thread settings " Nicolas Frey
2025-11-21 14:15   ` Christian Ebner
2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 2/4] verify: use tuning options' thread settings for {verify, read}-threads Nicolas Frey
2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 3/4] ui: tuning: make verify-job-{read, verify}-threads configurable Nicolas Frey
2025-11-21 12:32 ` [pbs-devel] [PATCH proxmox-backup 4/4] ui: verify: fetch default {read, verify} thread values from tuning options Nicolas Frey
2025-11-21 14:15 ` [pbs-devel] [RFC/PATCH proxmox{, -backup} 0/5] tuning: verify: add verify job thread options Christian Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal