fix(docs): make docs writes transactional and the group quota a bound
Audit #6 and #8, applied to the docs store — the same two bugs KV had, in the same two places. Per-app docs (#6): create/update/delete wrote the row, then emitted best-effort. An outbox failure left a committed doc whose trigger never fired. They now go through `atomic_write::DocsWriter`, whose Postgres impl writes and fans out on one connection in one transaction. Group docs (#8): the service read `count_rows`/`projected_total_bytes` on pooled connections and wrote on another, so concurrent creators each saw the same pre-write count and together overshot the ceiling. `PostgresGroupDocsWriter` takes a per-group advisory lock — on its OWN `docs`-namespaced key, so it serializes against other docs writers to that group but not against KV writers, which draw on a separate ceiling. The bespoke `check_total_bytes` (with its own copy of the upper-bound fast path) is gone; group docs now shares `group_quota::check_group_write` with group KV, so the row ceiling, the projected-bytes ceiling, and the fast path that skips the O(n) SUM scan have exactly one implementation between them. tests/atomic_write.rs gains the docs row-quota race (16 concurrent creators against a ceiling of 4). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -26,10 +26,10 @@ use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use picloud_shared::{
|
||||
DocId, DocRow, DocsError, DocsListPage, DocsService, SdkCallCx, ServiceEvent,
|
||||
ServiceEventEmitter,
|
||||
DocId, DocRow, DocsError, DocsListPage, DocsService, SdkCallCx, ServiceEventEmitter,
|
||||
};
|
||||
|
||||
use crate::atomic_write::{BestEffortDocsWriter, DocsWriter, PostgresDocsWriter};
|
||||
use crate::authz::{self, AuthzRepo, Capability};
|
||||
use crate::docs_filter::{parse_filter, FilterParseError};
|
||||
use crate::docs_repo::{DocsRepo, DocsRepoError};
|
||||
@@ -55,9 +55,11 @@ pub fn docs_max_value_bytes_from_env() -> usize {
|
||||
}
|
||||
|
||||
pub struct DocsServiceImpl {
|
||||
/// Reads only. Mutations go through `writer`, which owns the write AND the
|
||||
/// trigger fan-out so the two can share a transaction.
|
||||
repo: Arc<dyn DocsRepo>,
|
||||
authz: Arc<dyn AuthzRepo>,
|
||||
events: Arc<dyn ServiceEventEmitter>,
|
||||
writer: Arc<dyn DocsWriter>,
|
||||
max_value_bytes: usize,
|
||||
}
|
||||
|
||||
@@ -79,13 +81,23 @@ impl DocsServiceImpl {
|
||||
max_value_bytes: usize,
|
||||
) -> Self {
|
||||
Self {
|
||||
writer: Arc::new(BestEffortDocsWriter::new(repo.clone(), events)),
|
||||
repo,
|
||||
authz,
|
||||
events,
|
||||
max_value_bytes,
|
||||
}
|
||||
}
|
||||
|
||||
/// Swap the best-effort writer for the transactional one: the write and its
|
||||
/// trigger fan-out then commit together, so an outbox failure rolls the
|
||||
/// write back instead of silently losing the event. The host always calls
|
||||
/// this; the in-memory unit tests do not.
|
||||
#[must_use]
|
||||
pub fn with_atomic_writes(mut self, pool: sqlx::PgPool) -> Self {
|
||||
self.writer = Arc::new(PostgresDocsWriter::new(pool));
|
||||
self
|
||||
}
|
||||
|
||||
fn check_data_size(&self, data: &serde_json::Value) -> Result<(), DocsError> {
|
||||
let encoded_len = serde_json::to_vec(data)
|
||||
.map(|v| v.len())
|
||||
@@ -163,30 +175,7 @@ impl DocsService for DocsServiceImpl {
|
||||
validate_data(&data)?;
|
||||
self.check_data_size(&data)?;
|
||||
self.check_write(cx).await?;
|
||||
let row = self
|
||||
.repo
|
||||
.create(cx.app_id, collection, data.clone())
|
||||
.await?;
|
||||
// Best-effort emit — a failed emit logs but does not roll back
|
||||
// the write (mirrors KV's pattern).
|
||||
if let Err(e) = self
|
||||
.events
|
||||
.emit(
|
||||
cx,
|
||||
ServiceEvent {
|
||||
source: "docs",
|
||||
op: "create",
|
||||
collection: Some(collection.to_string()),
|
||||
key: Some(row.id.to_string()),
|
||||
payload: Some(data),
|
||||
old_payload: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::error!(error = %e, source = "docs", op = "create", event_emit_failure = true, "event emit failed");
|
||||
}
|
||||
Ok(row.id)
|
||||
self.writer.create(cx, collection, data).await
|
||||
}
|
||||
|
||||
async fn get(
|
||||
@@ -241,60 +230,17 @@ impl DocsService for DocsServiceImpl {
|
||||
validate_data(&data)?;
|
||||
self.check_data_size(&data)?;
|
||||
self.check_write(cx).await?;
|
||||
let previous = self
|
||||
.repo
|
||||
.update(cx.app_id, collection, id, data.clone())
|
||||
.await?;
|
||||
match previous {
|
||||
Some(prev) => {
|
||||
if let Err(e) = self
|
||||
.events
|
||||
.emit(
|
||||
cx,
|
||||
ServiceEvent {
|
||||
source: "docs",
|
||||
op: "update",
|
||||
collection: Some(collection.to_string()),
|
||||
key: Some(id.to_string()),
|
||||
payload: Some(data),
|
||||
old_payload: Some(prev),
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::error!(error = %e, source = "docs", op = "update", event_emit_failure = true, "event emit failed");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
None => Err(DocsError::NotFound),
|
||||
if self.writer.update(cx, collection, id, data).await? {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(DocsError::NotFound)
|
||||
}
|
||||
}
|
||||
|
||||
async fn delete(&self, cx: &SdkCallCx, collection: &str, id: DocId) -> Result<bool, DocsError> {
|
||||
validate_collection(collection)?;
|
||||
self.check_write(cx).await?;
|
||||
let previous = self.repo.delete(cx.app_id, collection, id).await?;
|
||||
let was_present = previous.is_some();
|
||||
if let Some(prev) = previous {
|
||||
if let Err(e) = self
|
||||
.events
|
||||
.emit(
|
||||
cx,
|
||||
ServiceEvent {
|
||||
source: "docs",
|
||||
op: "delete",
|
||||
collection: Some(collection.to_string()),
|
||||
key: Some(id.to_string()),
|
||||
payload: None,
|
||||
old_payload: Some(prev),
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::error!(error = %e, source = "docs", op = "delete", event_emit_failure = true, "event emit failed");
|
||||
}
|
||||
}
|
||||
Ok(was_present)
|
||||
self.writer.delete(cx, collection, id).await
|
||||
}
|
||||
|
||||
async fn list(
|
||||
|
||||
Reference in New Issue
Block a user