public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v2 3/4] partial fix #6049: datastore: use config fast-path in Drop
Date: Fri, 14 Nov 2025 16:05:43 +0100	[thread overview]
Message-ID: <20251114150544.224839-4-s.rufinatscha@proxmox.com> (raw)
In-Reply-To: <20251114150544.224839-1-s.rufinatscha@proxmox.com>

The Drop impl of DataStore re-read datastore.cfg to decide whether
the entry should be evicted from the in-process cache (based on
maintenance mode’s clear_from_cache). During the investigation of
issue #6049 [1], a flamegraph [2] showed that the config reload in Drop
accounted for a measurable share of CPU time under load.

This patch adds the datastore config fast path to the Drop impl to
eventually avoid an expensive config reload from disk to capture
the maintenance mandate.

Links

[1] Bugzilla: https://bugzilla.proxmox.com/show_bug.cgi?id=6049
[2] cargo-flamegraph: https://github.com/flamegraph-rs/flamegraph

Fixes: #6049
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 43 +++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index e7748872..0fabf592 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -214,15 +214,40 @@ impl Drop for DataStore {
             // remove datastore from cache iff
             //  - last task finished, and
             //  - datastore is in a maintenance mode that mandates it
-            let remove_from_cache = last_task
-                && pbs_config::datastore::config()
-                    .and_then(|(s, _)| s.lookup::<DataStoreConfig>("datastore", self.name()))
-                    .is_ok_and(|c| {
-                        c.get_maintenance_mode()
-                            .is_some_and(|m| m.clear_from_cache())
-                    });
-
-            if remove_from_cache {
+
+            // first check: check if last task finished
+            if !last_task {
+                return;
+            }
+
+            let (section_config, _gen) = match datastore_section_config_cached() {
+                Ok(v) => v,
+                Err(err) => {
+                    log::error!(
+                        "failed to load datastore config in Drop for {} - {err}",
+                        self.name()
+                    );
+                    return;
+                }
+            };
+
+            let datastore_cfg: DataStoreConfig =
+                match section_config.lookup("datastore", self.name()) {
+                    Ok(cfg) => cfg,
+                    Err(err) => {
+                        log::error!(
+                            "failed to look up datastore '{}' in Drop - {err}",
+                            self.name()
+                        );
+                        return;
+                    }
+                };
+
+            // second check: check maintenance mode mandate
+            if datastore_cfg
+                .get_maintenance_mode()
+                .is_some_and(|m| m.clear_from_cache())
+            {
                 DATASTORE_MAP.lock().unwrap().remove(self.name());
             }
         }
-- 
2.47.3



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

  parent reply	other threads:[~2025-11-14 15:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-14 15:05 [pbs-devel] [PATCH proxmox-backup v2 0/4] datastore: remove config reload on hot path Samuel Rufinatscha
2025-11-14 15:05 ` [pbs-devel] [PATCH proxmox-backup v2 1/4] partial fix #6049: config: enable config version cache for datastore Samuel Rufinatscha
2025-11-14 15:05 ` [pbs-devel] [PATCH proxmox-backup v2 2/4] partial fix #6049: datastore: impl ConfigVersionCache fast path for lookups Samuel Rufinatscha
2025-11-14 15:05 ` Samuel Rufinatscha [this message]
2025-11-14 15:05 ` [pbs-devel] [PATCH proxmox-backup v2 4/4] partial fix #6049: datastore: add TTL fallback to catch manual config edits Samuel Rufinatscha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251114150544.224839-4-s.rufinatscha@proxmox.com \
    --to=s.rufinatscha@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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