all lists on 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 3/3] datastore: add TTL fallback to catch manual config edits
Date: Tue, 11 Nov 2025 13:29:41 +0100	[thread overview]
Message-ID: <20251111122941.110412-4-s.rufinatscha@proxmox.com> (raw)
In-Reply-To: <20251111122941.110412-1-s.rufinatscha@proxmox.com>

The lookup fast path reacts to API-driven config changes because
save_config() bumps the generation. Manual edits of datastore.cfg do
not bump the counter. To keep the system robust against such edits
without reintroducing config reading and hashing on the hot path, this
patch adds a TTL to the cache entry.

If the datastore’s cached tag is older than
DATASTORE_CONFIG_CACHE_TTL_SECS (set to 60s), the next lookup takes
the slow path (re-read/parse) and refreshes the cached entry. Within
the TTL window, unchanged generations still use the fast path.

Note: Manual edits may remain unseen until the TTL elapses or any API
config write occurs.

Testing

With the TTL enabled, flamegraphs for hot status requests remain flat. A
0.1 second interval test confirmed periodic latency spikes at TTL expiry.

Maintainer notes

No dependency bumps or breaking changes.

Links

[1] cargo-flamegraph: https://github.com/flamegraph-rs/flamegraph

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

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index da80416a..5eaae49b 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -22,7 +22,7 @@ use proxmox_sys::error::SysError;
 use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
 use proxmox_sys::linux::procfs::MountInfo;
 use proxmox_sys::process_locker::{ProcessLockExclusiveGuard, ProcessLockSharedGuard};
-use proxmox_time::TimeSpan;
+use proxmox_time::{epoch_i64, TimeSpan};
 use proxmox_worker_task::WorkerTaskContext;
 
 use pbs_api_types::{
@@ -56,6 +56,9 @@ pub const GROUP_OWNER_FILE_NAME: &str = "owner";
 /// Filename for in-use marker stored on S3 object store backend
 pub const S3_DATASTORE_IN_USE_MARKER: &str = ".in-use";
 const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
+/// Max age in seconds to reuse the datastore lookup fast path
+/// before forcing a slow-path config read.
+const DATASTORE_CONFIG_CACHE_TTL_SECS: i64 = 60;
 
 /// checks if auth_id is owner, or, if owner is a token, if
 /// auth_id is the user of the token
@@ -254,6 +257,8 @@ struct CachedDatastoreConfigTag {
     last_maintenance_mode: Option<MaintenanceMode>,
     /// Datastore generation number from `ConfigVersionCache`; `None` when the cache wasn't available.
     last_generation: Option<usize>,
+    /// Epoch seconds when this lookup hint was created.
+    last_update: i64,
 }
 
 impl DataStore {
@@ -335,13 +340,16 @@ impl DataStore {
         let gen_num = ConfigVersionCache::new()
             .ok()
             .map(|c| c.datastore_generation());
+        let now = epoch_i64();
 
         // Fast-path: if we have a cached entry created under the same datastore.cfg generation number, reuse it.
         if let (Some(gen_num), Some(ds)) =
             (gen_num, DATASTORE_MAP.lock().unwrap().get(name).cloned())
         {
             if let Some(cached_tag) = &ds.cached_config_tag {
-                if cached_tag.last_generation == Some(gen_num) {
+                if cached_tag.last_generation == Some(gen_num)
+                    && (now - cached_tag.last_update) < DATASTORE_CONFIG_CACHE_TTL_SECS
+                {
                     if let Some(mm) = &cached_tag.last_maintenance_mode {
                         if let Err(error) = mm.check(operation) {
                             bail!("datastore '{name}' is unavailable: {error}");
@@ -397,6 +405,7 @@ impl DataStore {
         datastore.cached_config_tag = Some(CachedDatastoreConfigTag {
             last_maintenance_mode: maintenance_mode,
             last_generation: gen_num,
+            last_update: now,
         });
 
         let datastore = Arc::new(datastore);
-- 
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-11 12:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-11 12:29 [pbs-devel] [PATCH proxmox-backup 0/3] datastore: remove config reload on hot path Samuel Rufinatscha
2025-11-11 12:29 ` [pbs-devel] [PATCH proxmox-backup 1/3] partial fix #6049: datastore: impl ConfigVersionCache fast path for lookups Samuel Rufinatscha
2025-11-12 13:24   ` Fabian Grünbichler
2025-11-13 12:59     ` Samuel Rufinatscha
2025-11-11 12:29 ` [pbs-devel] [PATCH proxmox-backup 2/3] partial fix #6049: datastore: use config fast-path in Drop Samuel Rufinatscha
2025-11-12 11:24   ` Fabian Grünbichler
2025-11-12 15:20     ` Samuel Rufinatscha
2025-11-11 12:29 ` Samuel Rufinatscha [this message]
2025-11-12 11:27 ` [pbs-devel] [PATCH proxmox-backup 0/3] datastore: remove config reload on hot path Fabian Grünbichler
2025-11-12 17:27   ` Samuel Rufinatscha
2025-11-14 15:08 ` [pbs-devel] superseded: " 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=20251111122941.110412-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal