[pbs-devel] [PATCH proxmox-backup 2/3] partial fix #6049: datastore: use config fast-path in Drop

[Samuel Rufinatscha]

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 makes Drop O(1) on the fast path by reusing the maintenance-
mode decision captured at lookup time and stored with the cached
datastore entry. When the last reference goes away we:
- decrement active-operation counters, and
- evict only if the cached decision mandates eviction.

If the cache tag is absent or not fresh, a subsequent slow-path lookup
will be performed.

Testing

Compared flamegraphs before and after: prior to this change
(on top of patch 1), stacks originating from Drop included
pbs_config::datastore::config(). After the change, those vanish from
the drop path.

An end-to-end benchmark using `/status?verbose=0` with 1000 datastores,
5 requests per store, and 16-way parallelism shows a further
improvement:

| Metric                  | After commit 1 | After commit 2 | Δ (abs) | Δ (%)   |
|-------------------------|:--------------:|:--------------:|:-------:|:-------:|
| Total time              | 11s            | 10s            | −1s     | −9.09%  |
| Throughput (all rounds) | 454.55         | 500.00         | +45.45  | +10.00% |
| Cold RPS (round #1)     | 90.91          | 100.00         | +9.09   | +10.00% |
| Warm RPS (rounds 2..N)  | 363.64         | 400.00         | +36.36  | +10.00% |

Optimizing Drop improves overall throughput by ~10%. The gain appears
in both cold and warm rounds, and the flamegraph confirms the config
reload no longer sits on the hot path.

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 at proxmox.com>
---
 pbs-datastore/src/datastore.rs | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 18eebb58..da80416a 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -200,15 +200,38 @@ 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()
+
+            // first check: check if last task finished
+            if !last_task {
+                return;
+            }
+
+            let cached_tag = self.inner.cached_config_tag.as_ref();
+            let last_gen_num = cached_tag.and_then(|c| c.last_generation);
+            let gen_num = ConfigVersionCache::new()
+                .ok()
+                .map(|c| c.datastore_generation());
+
+            let cache_is_fresh = match (last_gen_num, gen_num) {
+                (Some(a), Some(b)) => a == b,
+                _ => false,
+            };
+
+            let mm_mandate = if cache_is_fresh {
+                cached_tag
+                    .and_then(|c| c.last_maintenance_mode.as_ref())
+                    .is_some_and(|m| m.clear_from_cache())
+            } else {
+                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 {
+            // second check: check maintenance mode mandate
+            if mm_mandate {
                 DATASTORE_MAP.lock().unwrap().remove(self.name());
             }
         }
-- 
2.47.3