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 v3 6/6] datastore: only bump generation when config digest changes
Date: Thu, 20 Nov 2025 14:03:42 +0100	[thread overview]
Message-ID: <20251120130342.248815-7-s.rufinatscha@proxmox.com> (raw)
In-Reply-To: <20251120130342.248815-1-s.rufinatscha@proxmox.com>

When reloading datastore.cfg in datastore_section_config_cached(),
we currently bump the datastore generation unconditionally. This is
only necessary when the on disk content actually changed and when
we already had a previous cached entry.

This patch extends the DatastoreConfigCache to store the last digest of
datastore.cfg and track the previously cached generation and digest.
Only when the digest differs from the cached one. On first load, it
reuses the existing datastore_generation without bumping.

This avoids unnecessary cache invalidations if the config did not
change.

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

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 12076f31..bf04332e 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -51,6 +51,8 @@ use crate::{DataBlob, LocalDatastoreLruCache};
 struct DatastoreConfigCache {
     // Parsed datastore.cfg file
     config: Arc<SectionConfigData>,
+    // Digest of the datastore.cfg file
+    last_digest: [u8; 32],
     // Generation number from ConfigVersionCache
     last_generation: usize,
     // Last update time (epoch seconds)
@@ -349,29 +351,44 @@ fn datastore_section_config_cached(
     }
 
     // Slow path: re-read datastore.cfg
-    let (config_raw, _digest) = pbs_config::datastore::config()?;
+    let (config_raw, digest) = pbs_config::datastore::config()?;
     let config = Arc::new(config_raw);
 
-    // Update cache
+    // Decide whether to bump the shared generation.
+    // Only bump if we already had a cached generation and the digest changed (manual edit or API write)
+    let (prev_gen, prev_digest) = guard
+        .as_ref()
+        .map(|c| (Some(c.last_generation), Some(c.last_digest)))
+        .unwrap_or((None, None));
+
     let new_gen = if let Some(handle) = version_cache {
-        // Bump datastore generation whenever we reload the config.
-        // This ensures that Drop handlers will detect that a newer config exists
-        // and will not rely on a stale cached entry for maintenance mandate.
-        let prev_gen = handle.increase_datastore_generation();
-        let new_gen = prev_gen + 1;
+        match (prev_gen, prev_digest) {
+            // We had a previous generation and the digest changed => bump generation.
+            (Some(_prev_gen), Some(prev_digest)) if prev_digest != digest => {
+                let old = handle.increase_datastore_generation();
+                Some(old + 1)
+            }
+            // We had a previous generation but the digest stayed the same:
+            // keep the existing generation, just refresh the timestamp.
+            (Some(prev_gen), _) => Some(prev_gen),
+            // We didn't have a previous generation, just use the current one.
+            (None, _) => Some(handle.datastore_generation()),
+        }
+    } else {
+        None
+    };
 
+    if let Some(gen_val) = new_gen {
         *guard = Some(DatastoreConfigCache {
             config: config.clone(),
-            last_generation: new_gen,
+            last_digest: digest,
+            last_generation: gen_val,
             last_update: now,
         });
-
-        Some(new_gen)
     } else {
-        // if the cache was not available, use again the slow path next time
+        // If the shared version cache is not available, don't cache.
         *guard = None;
-        None
-    };
+    }
 
     Ok((config, new_gen))
 }
-- 
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-20 13:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-20 13:03 [pbs-devel] [PATCH proxmox-backup v3 0/6] datastore: remove config reload on hot path Samuel Rufinatscha
2025-11-20 13:03 ` [pbs-devel] [PATCH proxmox-backup v3 1/6] partial fix #6049: config: enable config version cache for datastore Samuel Rufinatscha
2025-11-20 13:03 ` [pbs-devel] [PATCH proxmox-backup v3 2/6] partial fix #6049: datastore: impl ConfigVersionCache fast path for lookups Samuel Rufinatscha
2025-11-20 13:03 ` [pbs-devel] [PATCH proxmox-backup v3 3/6] partial fix #6049: datastore: use config fast-path in Drop Samuel Rufinatscha
2025-11-20 13:03 ` [pbs-devel] [PATCH proxmox-backup v3 4/6] partial fix #6049: datastore: add TTL fallback to catch manual config edits Samuel Rufinatscha
2025-11-20 13:03 ` [pbs-devel] [PATCH proxmox-backup v3 5/6] partial fix #6049: datastore: add reload flag to config cache helper Samuel Rufinatscha
2025-11-20 14:50   ` Fabian Grünbichler
2025-11-20 18:15     ` Samuel Rufinatscha
2025-11-20 13:03 ` Samuel Rufinatscha [this message]
2025-11-20 14:50   ` [pbs-devel] [PATCH proxmox-backup v3 6/6] datastore: only bump generation when config digest changes Fabian Grünbichler
2025-11-20 14:50 ` [pbs-devel] [PATCH proxmox-backup v3 0/6] datastore: remove config reload on hot path Fabian Grünbichler
2025-11-20 15:17   ` 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=20251120130342.248815-7-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