Cut recurring Argon2 cost and remove per-lock WebDAV timers
Two hot-path fixes for DAV sync clients, which poll continuously: 1. App-password Basic Auth cache TTL 30s → 300s. Every cache miss costs a full Argon2id verification (~50-100ms CPU) plus two DB queries; at 30s a continuously-syncing client re-paid that every 30s. 5 min cuts it ~10x. Safe because revoke() already invalidates the user's cached entries immediately; expiry/deactivation are only re-checked on a miss, so they now have a <=5 min grace window (comparable to a JWT access-token lifetime) — documented on the constant. 2. WebDAV lock store: replace the tokio::spawn + sleep scheduled per acquire/refresh with Moka's per-entry Expiry policy. Office clients refresh locks constantly, leaving thousands of orphaned sleeping timers pinned in the runtime that were never cancelled. by_path now carries the exact per-lock TTL via LockExpiry (no background tasks); by_token keeps a 24h backstop and resolves through by_path, so a lingering reverse-index entry can never resurrect an expired lock. Adds unit tests for the expiry policy and acquire/refresh/release (the module had none). https://claude.ai/code/session_0193Hff42gaA962wThxMGSd1
This commit is contained in:
@@ -1,15 +1,23 @@
|
||||
//! WebDAV lock store backed by Moka (in-memory cache with per-entry TTL).
|
||||
//!
|
||||
//! Locks are automatically evicted when their timeout expires, preventing
|
||||
//! orphaned locks from accumulating. Two caches are maintained:
|
||||
//! Each lock expires automatically at its own RFC 4918 `Timeout`, enforced
|
||||
//! by Moka's [`Expiry`](moka::Expiry) policy. There are **no background
|
||||
//! tasks and no per-lock timers** — Office clients refresh locks
|
||||
//! constantly, and spawning a `sleep` future per acquire/refresh used to
|
||||
//! leave thousands of orphaned timers pinned in the runtime. Two caches are
|
||||
//! maintained:
|
||||
//!
|
||||
//! - `by_path` : path → `LockEntry` (for LOCK conflict detection)
|
||||
//! - `by_token` : token → path (for fast UNLOCK / refresh lookups)
|
||||
//! - `by_path` : path → `LockEntry` (source of truth; precise per-lock TTL)
|
||||
//! - `by_token` : token → path (reverse index for UNLOCK / refresh)
|
||||
//!
|
||||
//! Both caches share the same TTL so entries disappear together.
|
||||
//! `by_path` carries the exact per-lock TTL via `Expiry`; `by_token` keeps a
|
||||
//! 24 h backstop TTL. A reverse-index entry that outlives its lock is
|
||||
//! harmless: every lookup resolves through `by_path`, which is
|
||||
//! authoritative, so an expired lock reads as absent even before its token
|
||||
//! mapping is evicted.
|
||||
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use crate::application::adapters::webdav_adapter::{LockInfo, LockScope};
|
||||
|
||||
@@ -26,6 +34,40 @@ pub struct LockEntry {
|
||||
pub path: String,
|
||||
}
|
||||
|
||||
/// Per-entry expiration policy for the `by_path` cache.
|
||||
///
|
||||
/// Moka calls this on insert (create) and re-insert (update, i.e. refresh)
|
||||
/// to derive each lock's TTL from its own `Timeout` header — replacing the
|
||||
/// old "global TTL + one spawned timer per lock" scheme. Reads do not
|
||||
/// extend the lock (the default `expire_after_read` leaves the remaining
|
||||
/// duration untouched).
|
||||
struct LockExpiry;
|
||||
|
||||
impl moka::Expiry<String, LockEntry> for LockExpiry {
|
||||
fn expire_after_create(
|
||||
&self,
|
||||
_path: &String,
|
||||
entry: &LockEntry,
|
||||
_created_at: Instant,
|
||||
) -> Option<Duration> {
|
||||
Some(WebDavLockStore::parse_timeout(
|
||||
entry.info.timeout.as_deref(),
|
||||
))
|
||||
}
|
||||
|
||||
fn expire_after_update(
|
||||
&self,
|
||||
_path: &String,
|
||||
entry: &LockEntry,
|
||||
_updated_at: Instant,
|
||||
_remaining: Option<Duration>,
|
||||
) -> Option<Duration> {
|
||||
Some(WebDavLockStore::parse_timeout(
|
||||
entry.info.timeout.as_deref(),
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
/// In-memory WebDAV lock store with automatic TTL-based expiration.
|
||||
///
|
||||
/// Uses Moka's `sync::Cache` — lock-free (sharded) reads, bounded size,
|
||||
@@ -42,13 +84,16 @@ impl WebDavLockStore {
|
||||
///
|
||||
/// * `max_capacity` — upper bound on simultaneous locks (evicts LRU on overflow).
|
||||
pub fn new(max_capacity: u64) -> Self {
|
||||
// We use `expire_after` (per-entry TTL) via insert with explicit ttl,
|
||||
// so we configure a generous global time_to_live as a safety net.
|
||||
// `by_path` is the source of truth: each lock expires at its own
|
||||
// `Timeout` via the `LockExpiry` policy (no spawned timers).
|
||||
let by_path = moka::sync::Cache::builder()
|
||||
.max_capacity(max_capacity)
|
||||
.time_to_live(Duration::from_secs(MAX_LOCK_TIMEOUT_SECS))
|
||||
.expire_after(LockExpiry)
|
||||
.build();
|
||||
|
||||
// `by_token` is a reverse index; a 24 h backstop TTL bounds any
|
||||
// mapping that outlives its lock. Lookups resolve through `by_path`,
|
||||
// so a lingering entry here never resurrects an expired lock.
|
||||
let by_token = moka::sync::Cache::builder()
|
||||
.max_capacity(max_capacity)
|
||||
.time_to_live(Duration::from_secs(MAX_LOCK_TIMEOUT_SECS))
|
||||
@@ -72,38 +117,17 @@ impl WebDavLockStore {
|
||||
return Err(existing);
|
||||
}
|
||||
|
||||
let ttl = Self::parse_timeout(info.timeout.as_deref());
|
||||
let entry = LockEntry {
|
||||
info,
|
||||
path: path.to_owned(),
|
||||
};
|
||||
|
||||
// `LockExpiry` derives the TTL from `entry.info.timeout` on insert —
|
||||
// no spawned timer needed.
|
||||
self.by_path.insert(path.to_owned(), entry.clone());
|
||||
self.by_token
|
||||
.insert(entry.info.token.clone(), path.to_owned());
|
||||
|
||||
// Moka 0.12 does not expose per-entry set_expiration_after_insert at
|
||||
// insert time. We rely on the global `time_to_live` as an upper bound
|
||||
// and use the `invalidate_after` helper below for custom TTL.
|
||||
//
|
||||
// To implement shorter-than-max TTL we schedule an async invalidation.
|
||||
if ttl.as_secs() < MAX_LOCK_TIMEOUT_SECS {
|
||||
let by_path = self.by_path.clone();
|
||||
let by_token = self.by_token.clone();
|
||||
let token = entry.info.token.clone();
|
||||
let path_owned = path.to_owned();
|
||||
tokio::spawn(async move {
|
||||
tokio::time::sleep(ttl).await;
|
||||
// Only remove if the entry still matches (wasn't refreshed/replaced)
|
||||
if let Some(e) = by_path.get(&path_owned)
|
||||
&& e.info.token == token
|
||||
{
|
||||
by_path.invalidate(&path_owned);
|
||||
by_token.invalidate(&token);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
Ok(entry)
|
||||
}
|
||||
|
||||
@@ -120,29 +144,13 @@ impl WebDavLockStore {
|
||||
}
|
||||
|
||||
let ttl = Self::parse_timeout(new_timeout.or(entry.info.timeout.as_deref()));
|
||||
let timeout_str = format!("Second-{}", ttl.as_secs());
|
||||
entry.info.timeout = Some(timeout_str.clone());
|
||||
// Normalize the stored timeout so `LockExpiry` recomputes the new TTL
|
||||
// from it on re-insert (Moka fires `expire_after_update`).
|
||||
entry.info.timeout = Some(format!("Second-{}", ttl.as_secs()));
|
||||
|
||||
// Re-insert to reset the TTL
|
||||
self.by_path.insert(path.clone(), entry.clone());
|
||||
self.by_token.insert(token.to_owned(), path.clone());
|
||||
|
||||
if ttl.as_secs() < MAX_LOCK_TIMEOUT_SECS {
|
||||
let by_path = self.by_path.clone();
|
||||
let by_token = self.by_token.clone();
|
||||
let token_owned = token.to_owned();
|
||||
let path_owned = path.clone();
|
||||
tokio::spawn(async move {
|
||||
tokio::time::sleep(ttl).await;
|
||||
if let Some(e) = by_path.get(&path_owned)
|
||||
&& e.info.token == token_owned
|
||||
{
|
||||
by_path.invalidate(&path_owned);
|
||||
by_token.invalidate(&token_owned);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
Some(entry)
|
||||
}
|
||||
|
||||
@@ -212,3 +220,136 @@ pub fn create_webdav_lock_store() -> Arc<WebDavLockStore> {
|
||||
// if the cap is reached, so stale entries are cleaned automatically.
|
||||
Arc::new(WebDavLockStore::new(10_000))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::application::adapters::webdav_adapter::LockType;
|
||||
use moka::Expiry;
|
||||
|
||||
fn lock_info(token: &str, timeout: Option<&str>, scope: LockScope) -> LockInfo {
|
||||
LockInfo {
|
||||
token: token.to_owned(),
|
||||
owner: Some("tester".to_owned()),
|
||||
depth: "0".to_owned(),
|
||||
timeout: timeout.map(str::to_owned),
|
||||
scope,
|
||||
type_: LockType::Write,
|
||||
}
|
||||
}
|
||||
|
||||
fn entry(token: &str, timeout: Option<&str>) -> LockEntry {
|
||||
LockEntry {
|
||||
info: lock_info(token, timeout, LockScope::Exclusive),
|
||||
path: "/file.txt".to_owned(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn expiry_uses_per_entry_timeout() {
|
||||
let now = Instant::now();
|
||||
let key = "/file.txt".to_owned();
|
||||
|
||||
// Explicit Second-NNN → that exact duration.
|
||||
let e = entry("t", Some("Second-300"));
|
||||
assert_eq!(
|
||||
LockExpiry.expire_after_create(&key, &e, now),
|
||||
Some(Duration::from_secs(300))
|
||||
);
|
||||
// Refresh path (update) recomputes from the (normalized) timeout.
|
||||
assert_eq!(
|
||||
LockExpiry.expire_after_update(&key, &e, now, None),
|
||||
Some(Duration::from_secs(300))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn expiry_clamps_infinite_and_defaults_none() {
|
||||
let now = Instant::now();
|
||||
let key = "/file.txt".to_owned();
|
||||
|
||||
let infinite = entry("t", Some("Infinite"));
|
||||
assert_eq!(
|
||||
LockExpiry.expire_after_create(&key, &infinite, now),
|
||||
Some(Duration::from_secs(MAX_LOCK_TIMEOUT_SECS))
|
||||
);
|
||||
|
||||
let none = entry("t", None);
|
||||
assert_eq!(
|
||||
LockExpiry.expire_after_create(&key, &none, now),
|
||||
Some(Duration::from_secs(DEFAULT_LOCK_TIMEOUT_SECS))
|
||||
);
|
||||
|
||||
// Over-large requests are clamped to the maximum.
|
||||
let huge = entry("t", Some("Second-999999999"));
|
||||
assert_eq!(
|
||||
LockExpiry.expire_after_create(&key, &huge, now),
|
||||
Some(Duration::from_secs(MAX_LOCK_TIMEOUT_SECS))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn acquire_get_release_roundtrip() {
|
||||
let store = WebDavLockStore::new(16);
|
||||
let info = lock_info("urn:token-1", Some("Second-600"), LockScope::Exclusive);
|
||||
|
||||
let acquired = store.acquire("/a.txt", info).expect("acquire");
|
||||
assert_eq!(acquired.info.token, "urn:token-1");
|
||||
|
||||
// Resolvable by both indexes.
|
||||
assert_eq!(
|
||||
store.get_by_path("/a.txt").map(|e| e.info.token.clone()),
|
||||
Some("urn:token-1".to_owned())
|
||||
);
|
||||
assert_eq!(
|
||||
store.get_by_token("urn:token-1").map(|e| e.path.clone()),
|
||||
Some("/a.txt".to_owned())
|
||||
);
|
||||
|
||||
assert!(store.release("urn:token-1"));
|
||||
assert!(store.get_by_path("/a.txt").is_none());
|
||||
assert!(store.get_by_token("urn:token-1").is_none());
|
||||
// Releasing an unknown token reports nothing removed.
|
||||
assert!(!store.release("urn:token-1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exclusive_lock_conflicts() {
|
||||
let store = WebDavLockStore::new(16);
|
||||
store
|
||||
.acquire(
|
||||
"/a.txt",
|
||||
lock_info("urn:token-1", Some("Second-600"), LockScope::Exclusive),
|
||||
)
|
||||
.expect("first acquire");
|
||||
|
||||
let conflict = store.acquire(
|
||||
"/a.txt",
|
||||
lock_info("urn:token-2", Some("Second-600"), LockScope::Exclusive),
|
||||
);
|
||||
assert!(conflict.is_err());
|
||||
// The original holder is returned so the caller can report it.
|
||||
assert_eq!(conflict.unwrap_err().info.token, "urn:token-1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refresh_normalizes_timeout_and_keeps_lock() {
|
||||
let store = WebDavLockStore::new(16);
|
||||
store
|
||||
.acquire(
|
||||
"/a.txt",
|
||||
lock_info("urn:token-1", Some("Infinite"), LockScope::Exclusive),
|
||||
)
|
||||
.expect("acquire");
|
||||
|
||||
let refreshed = store
|
||||
.refresh("urn:token-1", Some("Second-120"))
|
||||
.expect("refresh");
|
||||
assert_eq!(refreshed.info.timeout.as_deref(), Some("Second-120"));
|
||||
// Still present and still addressable by token.
|
||||
assert!(store.get_by_token("urn:token-1").is_some());
|
||||
|
||||
// Refreshing an unknown token yields None.
|
||||
assert!(store.refresh("urn:unknown", Some("Second-120")).is_none());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user