From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 3D5B71FF38E
	for <inbox@lore.proxmox.com>; Tue,  7 May 2024 09:30:26 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 37F0181D0;
	Tue,  7 May 2024 09:30:30 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Tue,  7 May 2024 09:29:54 +0200
Message-Id: <20240507072955.364206-3-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.39.2
In-Reply-To: <20240507072955.364206-1-d.csapak@proxmox.com>
References: <20240507072955.364206-1-d.csapak@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.016 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [datastore.rs, verify.rs]
Subject: [pbs-devel] [RFC PATCH proxmox-backup v2 2/3] verify: add tuning
 option for number of threads to use
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

a similar argument as for tape backups, but limited to the hashing part
of verification.

The ParallelHandler we use here only parallelizes the actual
verification, not the reading of chunks.

So in case the CPU is much slower than reading from storage, this could
help increase throughput.

In my benchmarks, the only difference i saw was when the cpu was limited
and I reduced the number of threads. So i guess this knob could help in
situations where the single-threaded read performance of the storage
outperforms the 4 cpu threads in hashing.

Because of that, I left the default at '4' threads like before.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 7 +++++++
 pbs-datastore/src/datastore.rs | 5 +++++
 src/backup/verify.rs           | 4 +++-
 www/Utils.js                   | 5 +++++
 www/datastore/OptionView.js    | 8 ++++++++
 5 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 1dae3867f..3fb9ff766 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -214,6 +214,11 @@ pub enum DatastoreFSyncLevel {
             optional: true,
             minimum: 1,
         },
+        "verification-threads": {
+            type: usize,
+            optional: true,
+            minimum: 1,
+        },
     },
 )]
 #[derive(Serialize, Deserialize, Default)]
@@ -228,6 +233,8 @@ pub struct DatastoreTuning {
     #[serde(skip_serializing_if = "Option::is_none")]
     /// Configures how many threads to use to read from the datastore while backing up to tape.
     pub tape_backup_read_threads: Option<usize>,
+    /// Configures how many threads to use for hashing on verify.
+    pub verification_threads: Option<usize>,
 }
 
 pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore tuning options")
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 73a1cfa39..b6552d92c 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -52,12 +52,14 @@ pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error>
 /// Contains the configuration of how many threads to use in various situations
 pub struct ThreadConfiguration {
     pub tape_backup_read_threads: usize,
+    pub verification_threads: usize,
 }
 
 impl Default for ThreadConfiguration {
     fn default() -> Self {
         Self {
             tape_backup_read_threads: 4,
+            verification_threads: 4,
         }
     }
 }
@@ -324,6 +326,9 @@ impl DataStore {
         if let Some(value) = tuning.tape_backup_read_threads {
             thread_config.tape_backup_read_threads = value;
         }
+        if let Some(value) = tuning.verification_threads {
+            thread_config.verification_threads = value;
+        }
 
         Ok(DataStoreImpl {
             chunk_store,
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index c972e5328..c9fbb2e33 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -123,9 +123,11 @@ fn verify_index_chunks(
     let verified_chunks2 = Arc::clone(&verify_worker.verified_chunks);
     let errors2 = Arc::clone(&errors);
 
+    let thread_count = datastore2.get_thread_configuration().verification_threads;
+
     let decoder_pool = ParallelHandler::new(
         "verify chunk decoder",
-        4,
+        thread_count,
         move |(chunk, digest, size): (DataBlob, [u8; 32], u64)| {
             let chunk_crypt_mode = match chunk.crypt_mode() {
                 Err(err) => {
diff --git a/www/Utils.js b/www/Utils.js
index 4d224cd4a..a94bdf86e 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -795,6 +795,11 @@ Ext.define('PBS.Utils', {
 	tapeBackupRT ??= Proxmox.Utils.defaultText + ` (4)`;
 	options.push(`${gettext('Tape Backup Read Threads')}: ${tapeBackupRT}`);
 
+	let verificationThreads = tuning['verification-threads'];
+	delete tuning['verification-threads'];
+	verificationThreads ??= Proxmox.Utils.defaultText + ` (4)`;
+	options.push(`${gettext('Verification Threads')}: ${verificationThreads}`);
+
 	for (const [k, v] of Object.entries(tuning)) {
 	    options.push(`${k}: ${v}`);
 	}
diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index cfbb89151..c28294bfd 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -279,6 +279,14 @@ Ext.define('PBS.Datastore.Options', {
 			    emptyText: '4',
 			    deleteEmpty: true,
 			},
+			{
+			    xtype: 'proxmoxintegerfield',
+			    name: 'verification-threads',
+			    fieldLabel: gettext('Verification Threads'),
+			    min: 1,
+			    emptyText: '4',
+			    deleteEmpty: true,
+			},
 		    ],
 		},
 	    },
-- 
2.39.2



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