fabi/PiCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(quota): measure the per-group byte projection with one consistent metric

Browse Source 
The §11.6 M4 byte quota mixed two incompatible byte measures: `used` came from
Postgres as `octet_length(value::text)` (jsonb CANONICAL text — whitespace after
`:` and `,`, keys normalized), while `old_len`/`new_len` were computed in Rust
with `serde_json::to_vec` (COMPACT, no spaces). The projection
`used - old_len + new_len` therefore subtracted and added a *smaller* measure
than what is actually stored, so a group could be admitted over its ceiling —
the cap drifted permissive. The two forms are not reconcilable in Rust (jsonb
also reorders/dedups keys), so the projection has to be measured by Postgres.

Add `projected_total_bytes` to `GroupKvRepo` (keyed by collection+key) and
`GroupDocsRepo` (keyed by the replaced doc id, `None` on create). Each computes
`SUM(canonical) - existing_row(canonical) + new_value(canonical)` in ONE query,
binding the new value as compact TEXT and re-parsing it through `::jsonb::text`
so PG measures it in exactly the form it stores. All three terms now share one
metric. The services call it instead of the mixed Rust math; the near-cap
fast-path (`rows_after * max_value_bytes <= ceiling` → skip the SUM) is kept, so
the common case still does no extra work. The docs `update` path no longer needs
its old-doc fetch (the subtraction happens in SQL), removing a read.

The in-memory test repos implement the same projection with their own (compact)
metric, so the quota unit tests keep their exact byte arithmetic. No schema
change.

Note: the check-then-write TOCTOU (concurrent writers can each pass and
collectively overshoot) is NOT addressed here — it needs a transaction spanning
the check and the write, the same missing infrastructure as the transactional
outbox. Tracked separately.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-07-14 07:18:14 +02:00
parent fc70f5e224
commit 155c5471b7
4 changed files with 170 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
crates/manager-core/src/group_kv_service.rs
View File

@@ -152,8 +152,9 @@ impl GroupKvServiceImpl {
}
/// Encode `value` and enforce the per-value byte cap, returning its encoded
/// length (both callers need it for the byte-quota projection). Mirrors the
/// sibling `group_docs_service::check_data_size`.
/// length. (The per-group byte quota measures canonically in SQL via
/// `projected_total_bytes`, so it no longer consumes this length.) Mirrors
/// the sibling `group_docs_service::check_data_size`.
fn check_value_size(&self, value: &serde_json::Value) -> Result<usize, GroupKvError> {
let encoded_len = serde_json::to_vec(value)
.map(|v| v.len())
@@ -219,7 +220,7 @@ impl GroupKvService for GroupKvServiceImpl {
value: serde_json::Value,
) -> Result<(), GroupKvError> {
let group_id = self.owning_group(cx, collection).await?;
let encoded_len = self.check_value_size(&value)?;
self.check_value_size(&value)?;
self.check_write(cx, group_id).await?;
// Determine insert vs update for the event op + quota deltas (best-effort,
// like the per-app path — the get+set is non-transactional but triggers
@@ -249,12 +250,13 @@ impl GroupKvService for GroupKvServiceImpl {
let rows_after = if existed { count } else { count + 1 };
let upper_bound = rows_after.saturating_mul(self.max_value_bytes as u64);
if upper_bound > self.max_total_bytes {
let old_len = existing
.as_ref()
.and_then(|v| serde_json::to_vec(v).ok())
.map_or(0u64, |b| b.len() as u64);
let used = self.repo.total_bytes(group_id).await?;
let projected = used.saturating_sub(old_len) + encoded_len as u64;
// Consistent-metric projection (canonical `octet_length(value::text)`
// for the current SUM, the old row, and the new value) — no
// Rust-compact vs DB-canonical drift.
let projected = self
.repo
.projected_total_bytes(group_id, collection, key, &value)
.await?;
if projected > self.max_total_bytes {
return Err(GroupKvError::TotalBytesQuotaExceeded {
limit: self.max_total_bytes,
@@ -286,7 +288,7 @@ impl GroupKvService for GroupKvServiceImpl {
new: serde_json::Value,
) -> Result<bool, GroupKvError> {
let group_id = self.owning_group(cx, collection).await?;
let encoded_len = self.check_value_size(&new)?;
self.check_value_size(&new)?;
self.check_write(cx, group_id).await?;
// Same quota rules as `set` (best-effort pre-check; the swap itself is
// atomic in the repo). A swap that inserts a NEW key (expected absent)
@@ -301,12 +303,10 @@ impl GroupKvService for GroupKvServiceImpl {
});
}
}
let old_len = existing
.as_ref()
.and_then(|v| serde_json::to_vec(v).ok())
.map_or(0u64, |b| b.len() as u64);
let used = self.repo.total_bytes(group_id).await?;
let projected = used.saturating_sub(old_len) + encoded_len as u64;
let projected = self
.repo
.projected_total_bytes(group_id, collection, key, &new)
.await?;
if projected > self.max_total_bytes {
return Err(GroupKvError::TotalBytesQuotaExceeded {
limit: self.max_total_bytes,
@@ -495,6 +495,27 @@ mod tests {
.map(|(_, v)| serde_json::to_vec(v).map_or(0, |b| b.len() as u64))
.sum())
}
/// Same projection the Postgres impl computes in SQL, but with this
/// mock's own (compact) metric: every OTHER row in the group, plus the
/// new value — i.e. `used - old + new`, measured consistently.
async fn projected_total_bytes(
&self,
group_id: GroupId,
collection: &str,
key: &str,
new_value: &serde_json::Value,
) -> Result<u64, GroupKvRepoError> {
let data = self.data.lock().await;
let target = (group_id, collection.to_string(), key.to_string());
let others: u64 = data
.iter()
.filter(|(k, _)| k.0 == group_id && **k != target)
.map(|(_, v)| serde_json::to_vec(v).map_or(0, |b| b.len() as u64))
.sum();
let new_len = serde_json::to_vec(new_value).map_or(0, |b| b.len() as u64);
Ok(others + new_len)
}
}
/// Maps `(app_id, lowercased name) -> owning group`. Anything absent is