From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id E95F61FF15C for ; Fri, 14 Nov 2025 16:05:44 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6194E16AF6; Fri, 14 Nov 2025 16:06:37 +0100 (CET) From: Samuel Rufinatscha To: pbs-devel@lists.proxmox.com Date: Fri, 14 Nov 2025 16:05:40 +0100 Message-ID: <20251114150544.224839-1-s.rufinatscha@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1763132735106 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.227 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible spam tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_MAILER 2 Automated Mailer Tag Left in Email SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup v2 0/4] datastore: remove config reload on hot path X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" Hi, this series reduces CPU time in datastore lookups by avoiding repeated datastore.cfg reads/parses in both `lookup_datastore()` and `DataStore::Drop`. It also adds a TTL so manual config edits are noticed without reintroducing hashing on every request. While investigating #6049 [1], cargo-flamegraph [2] showed hotspots during repeated `/status` calls in `lookup_datastore()` and in `Drop`, dominated by `pbs_config::datastore::config()` (config parse). The parsing cost itself should eventually be investigated in a future effort. Furthermore, cargo-flamegraph showed that when using a token-based auth method to access the API, a significant amount of time is spent in validation on every request request [3]. ## Approach [PATCH 1/4] Extend ConfigVersionCache for datastore generation Expose a dedicated datastore generation counter and an increment helper so callers can cheaply track datastore.cfg versions. [PATCH 2/4] Fast path for datastore lookups Cache the parsed datastore.cfg keyed by the shared datastore generation. lookup_datastore() reuses both the cached config and an existing DataStoreImpl when the generation matches, and falls back to the old slow path otherwise. [PATCH 3/4] Fast path for Drop Make DataStore::Drop use the cached config if possible instead of rereading datastore.cfg from disk. [PATCH 4/4] TTL to catch manual edits Add a small TTL around the cached config and bump the datastore generation whenever the config is reloaded. This catches manual edits to datastore.cfg without reintroducing hashing or config parsing on every request. ## Benchmark results All the following benchmarks are based on top of https://lore.proxmox.com/pbs-devel/20251112131525.645971-1-f.gruenbichler@proxmox.com/T/#u ### End-to-end Testing `/status?verbose=0` end-to-end with 1000 stores, 5 req/store and parallel=16 before/after the series: Metric Before After ---------------------------------------- Total time 12s 9s Throughput (all) 416.67 555.56 Cold RPS (round #1) 83.33 111.11 Warm RPS (#2..N) 333.33 444.44 Running under flamegraph [2], TLS appears to consume a significant amount of CPU time and blur the results. Still, a ~33% higher overall throughput and ~25% less end-to-end time for this workload. ### Isolated benchmarks (hyperfine) In addition to the end-to-end tests, I measured two standalone benchmarks with hyperfine, each using a config with 1000 datastores. `M` is the number of distinct datastores looked up and `N` is the number of lookups per datastore. Drop-direct variant: Drops the `DataStore` after every lookup, so the `Drop` path runs on every iteration: use anyhow::Error; use pbs_api_types::Operation; use pbs_datastore::DataStore; fn main() -> Result<(), Error> { let mut args = std::env::args(); args.next(); let datastores = if let Some(n) = args.next() { n.parse::()? } else { 1000 }; let iterations = if let Some(n) = args.next() { n.parse::()? } else { 1000 }; for d in 1..=datastores { let name = format!("ds{:04}", d); for i in 1..=iterations { DataStore::lookup_datastore(&name, Some(Operation::Write))?; } } Ok(()) } +----+------+-----------+-----------+---------+ | M | N | Baseline | Patched | Speedup | +----+------+-----------+-----------+---------+ | 1 | 1000 | 1.670 s | 34.3 ms | 48.7x | | 10 | 100 | 1.672 s | 34.5 ms | 48.4x | | 100| 10 | 1.679 s | 35.1 ms | 47.8x | |1000| 1 | 1.787 s | 38.2 ms | 46.8x | +----+------+-----------+-----------+---------+ Bulk-drop variant: Keeps the `DataStore` instances alive for all `N` lookups of a given datastore and then drops them in bulk, mimicking a task that performs many lookups while it is running and only triggers the expensive `Drop` logic when the last user exits. use anyhow::Error; use pbs_api_types::Operation; use pbs_datastore::DataStore; fn main() -> Result<(), Error> { let mut args = std::env::args(); args.next(); let datastores = if let Some(n) = args.next() { n.parse::()? } else { 1000 }; let iterations = if let Some(n) = args.next() { n.parse::()? } else { 1000 }; for d in 1..=datastores { let name = format!("ds{:04}", d); let mut stores = Vec::with_capacity(iterations); for i in 1..=iterations { stores.push(DataStore::lookup_datastore(&name, Some(Operation::Write))?); } } Ok(()) } +------+------+---------------+--------------+---------+ | M | N | Baseline mean | Patched mean | Speedup | +------+------+---------------+--------------+---------+ | 1 | 1000 | 884.0 ms | 33.9 ms | 26.1x | | 10 | 100 | 881.8 ms | 35.3 ms | 25.0x | | 100 | 10 | 969.3 ms | 35.9 ms | 27.0x | | 1000 | 1 | 1827.0 ms | 40.7 ms | 44.9x | +------+------+---------------+--------------+---------+ Both variants show that the combination of the cached config lookups and the cheaper `Drop` handling reduces the hot-path cost from ~1.7 s per run to a few tens of milliseconds in these benchmarks. ## Reproduction steps VM: 4 vCPU, ~8 GiB RAM, VirtIO-SCSI; disks: - scsi0 32G (OS) - scsi1 1000G (datastores) Install PBS from ISO on the VM. Set up ZFS on /dev/sdb (adjust if different): zpool create -f -o ashift=12 pbsbench /dev/sdb zfs set mountpoint=/pbsbench pbsbench zfs create pbsbench/pbs-bench Raise file-descriptor limit: sudo systemctl edit proxmox-backup-proxy.service Add the following lines: [Service] LimitNOFILE=1048576 Reload systemd and restart the proxy: sudo systemctl daemon-reload sudo systemctl restart proxmox-backup-proxy.service Verify the limit: systemctl show proxmox-backup-proxy.service | grep LimitNOFILE Create 1000 ZFS-backed datastores (as used in #6049 [1]): seq -w 001 1000 | xargs -n1 -P1 bash -c ' id=$0 name="ds${id}" dataset="pbsbench/pbs-bench/${name}" path="/pbsbench/pbs-bench/${name}" zfs create -o mountpoint="$path" "$dataset" proxmox-backup-manager datastore create "$name" "$path" \ --comment "ZFS dataset-based datastore" ' Build PBS from this series, then run the server under manually under flamegraph: systemctl stop proxmox-backup-proxy cargo flamegraph --release --bin proxmox-backup-proxy ## Other resources: ### E2E benchmark script: #!/usr/bin/env bash set -euo pipefail # --- Config --------------------------------------------------------------- HOST='https://localhost:8007' USER='root@pam' PASS="$(cat passfile)" DATASTORE_PATH="/pbsbench/pbs-bench" MAX_STORES=1000 # how many stores to include PARALLEL=16 # concurrent workers REPEAT=5 # requests per store (1 cold + REPEAT-1 warm) PRINT_FIRST=false # true => log first request's HTTP code per store # --- Helpers -------------------------------------------------------------- fmt_rps () { local n="$1" t="$2" awk -v n="$n" -v t="$t" 'BEGIN { if (t > 0) printf("%.2f\n", n/t); else print "0.00" }' } # --- Login --------------------------------------------------------------- auth=$(curl -ks -X POST "$HOST/api2/json/access/ticket" \ -d "username=$USER" -d "password=$PASS") ticket=$(echo "$auth" | jq -r '.data.ticket') if [[ -z "${ticket:-}" || "$ticket" == "null" ]]; then echo "[ERROR] Login failed (no ticket)" exit 1 fi # --- Collect stores (deterministic order) -------------------------------- mapfile -t STORES < <( find "$DATASTORE_PATH" -mindepth 1 -maxdepth 1 -type d -printf '%f\n' \ | sort | head -n "$MAX_STORES" ) USED_STORES=${#STORES[@]} if (( USED_STORES == 0 )); then echo "[ERROR] No datastore dirs under $DATASTORE_PATH" exit 1 fi echo "[INFO] Running with stores=$USED_STORES, repeat=$REPEAT, parallel=$PARALLEL" # --- Temp counters -------------------------------------------------------- SUCCESS_ALL="$(mktemp)" FAIL_ALL="$(mktemp)" COLD_OK="$(mktemp)" WARM_OK="$(mktemp)" trap 'rm -f "$SUCCESS_ALL" "$FAIL_ALL" "$COLD_OK" "$WARM_OK"' EXIT export HOST ticket REPEAT SUCCESS_ALL FAIL_ALL COLD_OK WARM_OK PRINT_FIRST SECONDS=0 # --- Fire requests -------------------------------------------------------- printf "%s\n" "${STORES[@]}" \ | xargs -P"$PARALLEL" -I{} bash -c ' store="$1" url="$HOST/api2/json/admin/datastore/$store/status?verbose=0" for ((i=1;i<=REPEAT;i++)); do code=$(curl -ks -o /dev/null -w "%{http_code}" -b "PBSAuthCookie=$ticket" "$url" || echo 000) if [[ "$code" == "200" ]]; then echo 1 >> "$SUCCESS_ALL" if (( i == 1 )); then echo 1 >> "$COLD_OK" else echo 1 >> "$WARM_OK" fi if [[ "$PRINT_FIRST" == "true" && $i -eq 1 ]]; then ts=$(date +%H:%M:%S) echo "[$ts] $store #$i HTTP:200" fi else echo 1 >> "$FAIL_ALL" if [[ "$PRINT_FIRST" == "true" && $i -eq 1 ]]; then ts=$(date +%H:%M:%S) echo "[$ts] $store #$i HTTP:$code (FAIL)" fi fi done ' _ {} # --- Summary -------------------------------------------------------------- elapsed=$SECONDS ok=$(wc -l < "$SUCCESS_ALL" 2>/dev/null || echo 0) fail=$(wc -l < "$FAIL_ALL" 2>/dev/null || echo 0) cold_ok=$(wc -l < "$COLD_OK" 2>/dev/null || echo 0) warm_ok=$(wc -l < "$WARM_OK" 2>/dev/null || echo 0) expected=$(( USED_STORES * REPEAT )) total=$(( ok + fail )) rps_all=$(fmt_rps "$ok" "$elapsed") rps_cold=$(fmt_rps "$cold_ok" "$elapsed") rps_warm=$(fmt_rps "$warm_ok" "$elapsed") echo "===== Summary =====" echo "Stores used: $USED_STORES" echo "Expected requests: $expected" echo "Executed requests: $total" echo "OK (HTTP 200): $ok" echo "Failed: $fail" printf "Total time: %dm %ds\n" $((elapsed/60)) $((elapsed%60)) echo "Throughput all RPS: $rps_all" echo "Cold RPS (round #1): $rps_cold" echo "Warm RPS (#2..N): $rps_warm" ## Maintainer notes No dependency bumps, no API changes and no breaking changes. ## Patch summary [PATCH 1/4] partial fix #6049: config: enable config version cache for datastore [PATCH 2/4] partial fix #6049: datastore: impl ConfigVersionCache fast path for lookups [PATCH 3/4] partial fix #6049: datastore: use config fast-path in Drop [PATCH 4/4] partial fix #6049: datastore: add TTL fallback to catch manual config edits Thanks, Samuel [1] Bugzilla #6049: https://bugzilla.proxmox.com/show_bug.cgi?id=6049 [2] cargo-flamegraph: https://github.com/flamegraph-rs/flamegraph [3] Bugzilla #7017: https://bugzilla.proxmox.com/show_bug.cgi?id=7017 Samuel Rufinatscha (4): partial fix #6049: config: enable config version cache for datastore partial fix #6049: datastore: impl ConfigVersionCache fast path for lookups partial fix #6049: datastore: use config fast-path in Drop partial fix #6049: datastore: add TTL fallback to catch manual config edits pbs-config/src/config_version_cache.rs | 10 +- pbs-datastore/Cargo.toml | 1 + pbs-datastore/src/datastore.rs | 187 +++++++++++++++++++------ 3 files changed, 152 insertions(+), 46 deletions(-) -- 2.47.3 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel