perf(authz): cache resource owner lookups in PgAclEngine
The owner short-circuit in PgAclEngine::check ran a PK query (SELECT user_id FROM storage.folders/files WHERE id=$1) on every authorization check of a folder/file — the common case, since users mostly act on their own resources. Memoise it in an owner_cache (moka, TTL 300s, 100k cap). The owner column is immutable, so this is safe: the cache maps resource -> real owner and can never grant a non-owner access (a different caller's owner==uid test fails against the cached owner and falls through to grants); a hard-deleted resource that briefly resolves to its former owner simply fails later at execution with NotFound. The per-check sql_queries counter now increments only on a miss. Removes 1 DB query + 1 pool-connection acquisition per owner check. Magnitude is deployment-specific (query latency x whether the pool is contended); see benches/ACL-OWNER-CACHE.md. Also adds two DB perf-investigation harnesses, gated behind the `bench` feature (need the dev Postgres; zero prod impact): - examples/bench_db_pool.rs + benches/DB-POOL.md — pool size vs tail latency - examples/bench_owner_cache.rs + benches/ACL-OWNER-CACHE.md — owner query vs cache Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -74,6 +74,13 @@ struct QueryCounters {
|
||||
/// the cap signals pathological data and is surfaced to operators via audit.
|
||||
const MAX_GRANT_ROWS: i64 = 10_000;
|
||||
|
||||
/// `owner_cache` bound: entries are tiny (Resource + Uuid). 100k ≈ a few MB.
|
||||
const OWNER_CACHE_CAPACITY: u64 = 100_000;
|
||||
/// `owner_cache` TTL. A resource's owner is immutable, so the only staleness is
|
||||
/// a hard-deleted resource briefly resolving to its former owner — harmless
|
||||
/// (see `owner_cache` field doc), hence a generous TTL for a high hit rate.
|
||||
const OWNER_CACHE_TTL: Duration = Duration::from_secs(300);
|
||||
|
||||
pub struct PgAclEngine {
|
||||
pool: Arc<PgPool>,
|
||||
folder_repo: Arc<FolderDbRepository>,
|
||||
@@ -84,6 +91,14 @@ pub struct PgAclEngine {
|
||||
/// entries; eviction is LRU + TTL. Stale by up to TTL after a membership
|
||||
/// change — acceptable trade-off (see plan, "Cache TTL behaviour").
|
||||
user_groups_cache: Cache<Uuid, Arc<HashSet<Uuid>>>,
|
||||
/// Memoise `resource → owner UUID`. The owner column is immutable, so the
|
||||
/// owner-short-circuit (the common case: a user touching their own files)
|
||||
/// no longer issues a PK query on every authorization check — just the first
|
||||
/// per resource within the TTL. **Safe**: this can never grant a non-owner
|
||||
/// access (a different caller's `owner == uid` test fails against the cached
|
||||
/// *real* owner), and a hard-deleted resource that briefly short-circuits as
|
||||
/// owned simply fails later at execution with NotFound.
|
||||
owner_cache: Cache<Resource, Uuid>,
|
||||
}
|
||||
|
||||
impl PgAclEngine {
|
||||
@@ -102,6 +117,10 @@ impl PgAclEngine {
|
||||
.max_capacity(50_000)
|
||||
.time_to_live(Duration::from_secs(30))
|
||||
.build(),
|
||||
owner_cache: Cache::builder()
|
||||
.max_capacity(OWNER_CACHE_CAPACITY)
|
||||
.time_to_live(OWNER_CACHE_TTL)
|
||||
.build(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -153,6 +172,10 @@ impl PgAclEngine {
|
||||
.max_capacity(1)
|
||||
.time_to_live(Duration::from_secs(1))
|
||||
.build(),
|
||||
owner_cache: Cache::builder()
|
||||
.max_capacity(1)
|
||||
.time_to_live(Duration::from_secs(1))
|
||||
.build(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -273,6 +296,23 @@ impl PgAclEngine {
|
||||
self.subject_match_set(subject, &counters).await
|
||||
}
|
||||
|
||||
/// Owner lookup with memoisation. Hits the DB only on a cache miss; the
|
||||
/// result is cached because a resource's owner never changes. `NotFound`
|
||||
/// (a hard-deleted / nonexistent resource) is propagated, not cached.
|
||||
async fn owner_of_cached(
|
||||
&self,
|
||||
resource: Resource,
|
||||
counters: &QueryCounters,
|
||||
) -> Result<Uuid, DomainError> {
|
||||
if let Some(owner) = self.owner_cache.get(&resource).await {
|
||||
return Ok(owner);
|
||||
}
|
||||
counters.sql_queries.fetch_add(1, Ordering::Relaxed);
|
||||
let owner = self.owner_of(resource).await?;
|
||||
self.owner_cache.insert(resource, owner).await;
|
||||
Ok(owner)
|
||||
}
|
||||
|
||||
/// Returns the owner UUID for any resource type.
|
||||
async fn owner_of(&self, resource: Resource) -> Result<Uuid, DomainError> {
|
||||
match resource {
|
||||
@@ -552,8 +592,7 @@ impl PgAclEngine {
|
||||
// there's no analogous fast path: the grant lookup below resolves
|
||||
// a drive owner via the same query that resolves any drive role.
|
||||
if let (Subject::User(uid), Resource::Folder(_) | Resource::File(_)) = (subject, resource) {
|
||||
counters.sql_queries.fetch_add(1, Ordering::Relaxed);
|
||||
match self.owner_of(resource).await {
|
||||
match self.owner_of_cached(resource, counters).await {
|
||||
Ok(owner) if owner == uid => return Ok(true),
|
||||
Ok(_) => { /* not owner — fall through to grants */ }
|
||||
Err(e) if e.kind == crate::common::errors::ErrorKind::NotFound => {
|
||||
|
||||
Reference in New Issue
Block a user