[pbs-devel] [PATCH proxmox-backup v6 2/4] partial fix #6049: datastore: impl ConfigVersionCache fast path for lookups
Samuel Rufinatscha
s.rufinatscha at proxmox.com
Mon Jan 5 15:16:12 CET 2026
Repeated /status requests caused lookup_datastore() to re-read and
parse datastore.cfg on every call. The issue was mentioned in report
#6049 [1]. cargo-flamegraph [2] confirmed that the hot path is
dominated by pbs_config::datastore::config() (config parsing).
This patch implements caching of the global datastore.cfg using the
generation numbers from the shared config version cache. It caches the
datastore.cfg along with the generation number and, when a subsequent
lookup sees the same generation, it reuses the cached config without
re-reading it from disk. If the generation differs
(or the cache is unavailable), the config is re-read from disk.
If `update_cache = true`, the new config and current generation are
persisted in the cache. In this case, callers must hold the datastore
config lock to avoid racing with concurrent config changes.
If `update_cache` is `false` and generation did not match, the freshly
read config is returned but the cache is left unchanged. If
`ConfigVersionCache` is not available, the config is always read from
disk and `None` is returned as generation.
Behavioral notes
- The generation is bumped via the existing save_config() path, so
API-driven config changes are detected immediately.
- Manual edits to datastore.cfg are not detected; this is covered in a
dedicated patch in this series.
- DataStore::drop still performs a config read on the common path;
also covered in a dedicated patch in this series.
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>
---
Changes:
>From v1 → v2
- Moved the ConfigVersionCache changes into its own patch,
thanks @Fabian
- Introduced the global static DATASTORE_CONFIG_CACHE to store the
fully parsed datastore.cfg instead, along with its generation number.
thanks @Fabian
- Introduced DatastoreConfigCache struct to hold cache values
- Removed and replaced the CachedDatastoreConfigTag field of
DataStoreImpl with a generation number field only (Option<usize>)
to validate DataStoreImpl reuse.
- Added DataStore::datastore_section_config_cached() helper function
to encapsulate the caching logic and simplify reuse.
- Modified DataStore::lookup_datastore() to use the new helper.
>From v2 → v3
No changes
>From v3 → v4, thanks @Fabian
- Restructured the version cache checks in
datastore_section_config_cached(), to simplify the logic.
- Added update_cache parameter to datastore_section_config_cached() to
control cache updates.
>From v4 → v5
- Rebased only, no changes
>From v5 → v6
- Rebased
- Styling: minimize/avoid diff noise, thanks @Fabian
pbs-datastore/Cargo.toml | 1 +
pbs-datastore/src/datastore.rs | 90 ++++++++++++++++++++++++++++------
2 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/pbs-datastore/Cargo.toml b/pbs-datastore/Cargo.toml
index 8ce930a9..42f49a7b 100644
--- a/pbs-datastore/Cargo.toml
+++ b/pbs-datastore/Cargo.toml
@@ -40,6 +40,7 @@ proxmox-io.workspace = true
proxmox-lang.workspace=true
proxmox-s3-client = { workspace = true, features = [ "impl" ] }
proxmox-schema = { workspace = true, features = [ "api-macro" ] }
+proxmox-section-config.workspace = true
proxmox-serde = { workspace = true, features = [ "serde_json" ] }
proxmox-sys.workspace = true
proxmox-systemd.workspace = true
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 9c57aaac..aa366826 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -34,7 +34,8 @@ use pbs_api_types::{
MaintenanceType, Operation, UPID,
};
use pbs_config::s3::S3_CFG_TYPE_ID;
-use pbs_config::BackupLockGuard;
+use pbs_config::{BackupLockGuard, ConfigVersionCache};
+use proxmox_section_config::SectionConfigData;
use crate::backup_info::{
BackupDir, BackupGroup, BackupInfo, OLD_LOCKING, PROTECTED_MARKER_FILENAME,
@@ -48,6 +49,17 @@ use crate::s3::S3_CONTENT_PREFIX;
use crate::task_tracking::{self, update_active_operations};
use crate::{DataBlob, LocalDatastoreLruCache};
+// Cache for fully parsed datastore.cfg
+struct DatastoreConfigCache {
+ // Parsed datastore.cfg file
+ config: Arc<SectionConfigData>,
+ // Generation number from ConfigVersionCache
+ last_generation: usize,
+}
+
+static DATASTORE_CONFIG_CACHE: LazyLock<Mutex<Option<DatastoreConfigCache>>> =
+ LazyLock::new(|| Mutex::new(None));
+
static DATASTORE_MAP: LazyLock<Mutex<HashMap<String, Arc<DataStoreImpl>>>> =
LazyLock::new(|| Mutex::new(HashMap::new()));
@@ -149,11 +161,13 @@ pub struct DataStoreImpl {
last_gc_status: Mutex<GarbageCollectionStatus>,
verify_new: bool,
chunk_order: ChunkOrder,
- last_digest: Option<[u8; 32]>,
sync_level: DatastoreFSyncLevel,
backend_config: DatastoreBackendConfig,
lru_store_caching: Option<LocalDatastoreLruCache>,
thread_settings: DatastoreThreadSettings,
+ /// datastore.cfg cache generation number at lookup time, used to
+ /// invalidate this cached `DataStoreImpl`
+ config_generation: Option<usize>,
}
impl DataStoreImpl {
@@ -166,11 +180,11 @@ impl DataStoreImpl {
last_gc_status: Mutex::new(GarbageCollectionStatus::default()),
verify_new: false,
chunk_order: Default::default(),
- last_digest: None,
sync_level: Default::default(),
backend_config: Default::default(),
lru_store_caching: None,
thread_settings: Default::default(),
+ config_generation: None,
})
}
}
@@ -286,6 +300,55 @@ impl DatastoreThreadSettings {
}
}
+/// Returns the parsed datastore config (`datastore.cfg`) and its
+/// generation.
+///
+/// Uses `ConfigVersionCache` to detect stale entries:
+/// - If the cached generation matches the current generation, the
+/// cached config is returned.
+/// - Otherwise the config is re-read from disk. If `update_cache` is
+/// `true`, the new config and current generation are stored in the
+/// cache. Callers that set `update_cache = true` must hold the
+/// datastore config lock to avoid racing with concurrent config
+/// changes.
+/// - If `update_cache` is `false`, the freshly read config is returned
+/// but the cache is left unchanged.
+///
+/// If `ConfigVersionCache` is not available, the config is always read
+/// from disk and `None` is returned as the generation.
+fn datastore_section_config_cached(
+ update_cache: bool,
+) -> Result<(Arc<SectionConfigData>, Option<usize>), Error> {
+ let mut config_cache = DATASTORE_CONFIG_CACHE.lock().unwrap();
+
+ if let Ok(version_cache) = ConfigVersionCache::new() {
+ let current_gen = version_cache.datastore_generation();
+ if let Some(cached) = config_cache.as_ref() {
+ // Fast path: re-use cached datastore.cfg
+ if cached.last_generation == current_gen {
+ return Ok((cached.config.clone(), Some(cached.last_generation)));
+ }
+ }
+ // Slow path: re-read datastore.cfg
+ let (config_raw, _digest) = pbs_config::datastore::config()?;
+ let config = Arc::new(config_raw);
+
+ if update_cache {
+ *config_cache = Some(DatastoreConfigCache {
+ config: config.clone(),
+ last_generation: current_gen,
+ });
+ }
+
+ Ok((config, Some(current_gen)))
+ } else {
+ // Fallback path, no config version cache: read datastore.cfg and return None as generation
+ *config_cache = None;
+ let (config_raw, _digest) = pbs_config::datastore::config()?;
+ Ok((Arc::new(config_raw), None))
+ }
+}
+
impl DataStore {
// This one just panics on everything
#[doc(hidden)]
@@ -367,10 +430,9 @@ impl DataStore {
// we use it to decide whether it is okay to delete the datastore.
let _config_lock = pbs_config::datastore::lock_config()?;
- // we could use the ConfigVersionCache's generation for staleness detection, but we load
- // the config anyway -> just use digest, additional benefit: manual changes get detected
- let (config, digest) = pbs_config::datastore::config()?;
- let config: DataStoreConfig = config.lookup("datastore", name)?;
+ // Get the current datastore.cfg generation number and cached config
+ let (section_config, gen_num) = datastore_section_config_cached(true)?;
+ let config: DataStoreConfig = section_config.lookup("datastore", name)?;
if let Some(maintenance_mode) = config.get_maintenance_mode() {
if let Err(error) = maintenance_mode.check(operation) {
@@ -378,19 +440,19 @@ impl DataStore {
}
}
+ let mut datastore_cache = DATASTORE_MAP.lock().unwrap();
+
if get_datastore_mount_status(&config) == Some(false) {
- let mut datastore_cache = DATASTORE_MAP.lock().unwrap();
datastore_cache.remove(&config.name);
bail!("datastore '{}' is not mounted", config.name);
}
- let mut datastore_cache = DATASTORE_MAP.lock().unwrap();
let entry = datastore_cache.get(name);
// reuse chunk store so that we keep using the same process locker instance!
let chunk_store = if let Some(datastore) = &entry {
- let last_digest = datastore.last_digest.as_ref();
- if let Some(true) = last_digest.map(|last_digest| last_digest == &digest) {
+ // Re-use DataStoreImpl
+ if datastore.config_generation == gen_num && gen_num.is_some() {
if let Some(operation) = operation {
update_active_operations(name, operation, 1)?;
}
@@ -412,7 +474,7 @@ impl DataStore {
)?)
};
- let datastore = DataStore::with_store_and_config(chunk_store, config, Some(digest))?;
+ let datastore = DataStore::with_store_and_config(chunk_store, config, gen_num)?;
let datastore = Arc::new(datastore);
datastore_cache.insert(name.to_string(), datastore.clone());
@@ -514,7 +576,7 @@ impl DataStore {
fn with_store_and_config(
chunk_store: Arc<ChunkStore>,
config: DataStoreConfig,
- last_digest: Option<[u8; 32]>,
+ generation: Option<usize>,
) -> Result<DataStoreImpl, Error> {
let mut gc_status_path = chunk_store.base_path();
gc_status_path.push(".gc-status");
@@ -579,11 +641,11 @@ impl DataStore {
last_gc_status: Mutex::new(gc_status),
verify_new: config.verify_new.unwrap_or(false),
chunk_order: tuning.chunk_order.unwrap_or_default(),
- last_digest,
sync_level: tuning.sync_level.unwrap_or_default(),
backend_config,
lru_store_caching,
thread_settings,
+ config_generation: generation,
})
}
--
2.47.3
More information about the pbs-devel
mailing list