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 5/6] partial fix #6049: datastore: add reload flag to config cache helper
Date: Thu, 20 Nov 2025 14:03:41 +0100	[thread overview]
Message-ID: <20251120130342.248815-6-s.rufinatscha@proxmox.com> (raw)
In-Reply-To: <20251120130342.248815-1-s.rufinatscha@proxmox.com>

Extend datastore_section_config_cached() with an `allow_reload` flag to
separate two use cases:

1) lookup_datastore() passes `true` and is allowed to reload
  datastore.cfg from disk when the cache is missing, the generation
  changed or the TTL expired. The helper may bump the datastore
  generation if the digest changed.

2) DataStore::drop() passes `false` and only consumes the most recent
  cached entry without touching the disk, TTL or generation. If the
  cache was never initialised, it returns an error.

This avoids races between Drop and concurrent config changes.

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

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 1711c753..12076f31 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -226,7 +226,7 @@ impl Drop for DataStore {
                 return;
             }
 
-            let (section_config, _gen) = match datastore_section_config_cached() {
+            let (section_config, _gen) = match datastore_section_config_cached(false) {
                 Ok(v) => v,
                 Err(err) => {
                     log::error!(
@@ -299,14 +299,42 @@ impl DatastoreBackend {
     }
 }
 
-/// Return the cached datastore SectionConfig and its generation.
-fn datastore_section_config_cached() -> Result<(Arc<SectionConfigData>, Option<usize>), Error> {
+/// Returns the cached `datastore.cfg` and its generation.
+///
+/// When `allow_reload` is `true`, callers are expected to hold the datastore config. It may:
+///   - Reload `datastore.cfg` from disk if either
+///       - no cache exists yet, or cache is unavailable
+///       - the cached generation does not match the shared generation
+///       - the cache entry is older than `DATASTORE_CONFIG_CACHE_TTL_SECS`
+///   - Updates the cache with the new config, timestamp and digest.
+///   - Bumps the datastore generation in `ConfigVersionCache` only if
+///     there was a previous cached entry and the digest changed (manual edit or
+///     API write). If the digest is unchanged, the timestamp is refreshed but the
+///     generation is kept to avoid unnecessary invalidations.
+///
+/// When `allow_reload` is `false`:
+///   - Never touches the disk or the shared generation.
+///   - Ignores TTL and simply returns the most recent cached entry if available.
+///   - Returns an error if the cache has not been initialised yet.
+///
+/// Intended for use with `Datastore::drop` where no config lock is held
+/// and eventual stale data is acceptable.
+fn datastore_section_config_cached(
+    allow_reload: bool,
+) -> Result<(Arc<SectionConfigData>, Option<usize>), Error> {
     let now = epoch_i64();
     let version_cache = ConfigVersionCache::new().ok();
     let current_gen = version_cache.as_ref().map(|c| c.datastore_generation());
 
     let mut guard = DATASTORE_CONFIG_CACHE.lock().unwrap();
 
+    if !allow_reload {
+        if let Some(cache) = guard.as_ref() {
+            return Ok((cache.config.clone(), Some(cache.last_generation)));
+        }
+        bail!("datastore config cache not initialized");
+    }
+
     // Fast path: re-use cached datastore.cfg if cache is available, generation matches and TTL not expired
     if let (Some(current_gen), Some(config_cache)) = (current_gen, guard.as_ref()) {
         let gen_matches = config_cache.last_generation == current_gen;
@@ -423,7 +451,7 @@ impl DataStore {
         let _config_lock = pbs_config::datastore::lock_config()?;
 
         // Get the current datastore.cfg generation number and cached config
-        let (section_config, gen_num) = datastore_section_config_cached()?;
+        let (section_config, gen_num) = datastore_section_config_cached(true)?;
 
         let datastore_cfg: DataStoreConfig = section_config.lookup("datastore", name)?;
         let maintenance_mode = datastore_cfg.get_maintenance_mode();
-- 
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:03 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 ` Samuel Rufinatscha [this message]
2025-11-20 14:50   ` [pbs-devel] [PATCH proxmox-backup v3 5/6] partial fix #6049: datastore: add reload flag to config cache helper Fabian Grünbichler
2025-11-20 18:15     ` Samuel Rufinatscha
2025-11-20 13:03 ` [pbs-devel] [PATCH proxmox-backup v3 6/6] datastore: only bump generation when config digest changes Samuel Rufinatscha
2025-11-20 14:50   ` 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-6-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