* ✨ Add storage operation metrics for S3 and buckets
Expose Prometheus metrics for the object storage subsystem.
The S3 backend now attaches an AWS SDK MetricPublisher that counts
API calls, retries and latency per operation and target. The storage
layer counts logical operations and deduplication outcomes per Penpot
bucket, and the assets handlers count served requests per route.
Closes#11676
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Fix storage metrics labels, errors and test gaps
Address the review findings on the storage metrics commit.
Label reads with the object's own backend, count failed asset
serving as errors without swallowing them, and cover the failed
S3 call, S3 asset path and permission-denied branches with tests.
Also share the label helper and reuse the metrics test helper.
Closes#11676
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Harden storage metrics and fill test gaps
Address the second-round review findings on storage metrics.
Unknown backends now fail explicitly and count as errors, exists
stays paired with its dedup outcome, and the thumbnail, missing
storage, expired reads, unknown buckets and write failure paths
are covered by tests. Label coercion goes through the shared
metrics helper.
Closes#11676
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Harden storage metrics accuracy and coverage
Address the full-branch review findings on storage metrics.
Touch and delete emit only on changed rows, reads emit after the
backend fetch, unknown backends fail explicitly, and tempfile
mismatches count as unauthorized. Publisher nil policy, pairing
rules and attempt semantics are documented and covered by tests.
Closes#11676
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Address full-branch review findings on storage metrics
Touch and delete resolve labels from the row, reads stay paired,
failures are covered by tests, and logging, ranges and docs are
tightened. Includes the label helper unit tests and the retries
wording clarification.
Closes#11676
AI-assisted-by: muse-spark-1.3-contributor
* ⚡ Label touch and del metrics from UPDATE RETURNING
The storage metrics change resolved metric labels for touch-object!
and del-object! with an extra SELECT per id-based call. Since app.main
instruments storage unconditionally, every GC collector and binfile
import paid that extra round trip: deleting a team with 10k media
objects doubled the storage_object statements exactly on the paths
that already process the most rows.
touch-object! and del-object! now take only the object id (UUID) and
read the labels from the updated row itself via RETURNING id, backend,
metadata: one statement, no pre-read, and labels that always match the
row actually mutated. del-object! additionally guards on deleted_at
IS NULL, so a repeated delete returns false and emits no metric.
Also from the review of the full branch: extract the duplicated
serve/emit/rethrow block in app.http.assets into one helper; give
penpot_storage_s3_timing explicit histogram buckets up to 60s (the
default cap at 7.5s hid the slow S3 calls the metric exists for);
drop the unused ::target-id config key from the S3 backend and
hardcode the :default target label until per-bucket routing lands.
AI-assisted-by: glm-5.3-flash
* ✨ Harden storage metric recording and definitions
The metric definition schema is now closed and declares every key
the collectors read: buckets, quantiles, max-age and reg. A typo
such as a misspelled ::mdef/buckets used to compile and silently
fall back to the default histogram buckets; it now fails at
startup.
The asset result-label fallback coerced an absent status to 500,
so a future serve path without a status would have counted
successes as errors. The mapping is now explicit and documented:
served below 400, unauthorized for 401/403, not-found for 404,
and error for everything else, including an absent status.
The never-fail try/catch around metric recording existed four
times with drift. One app.metrics/run-safe! helper replaces them:
it no-ops on a nil metrics instance and logs the first failure
per hint at warn level, then at debug, so a broken setup surfaces
once without flooding the log. The S3 publisher keeps its outer
try/catch: it is the SDK MetricPublisher contract boundary.
AI-assisted-by: glm-5.3-flash
* ✨ Make metrics mandatory and run! safe by default
Recording a metric must never change the behavior of the operation
being measured, so `run!` now catches recording failures itself: the
first failure per metric id logs at warn, later ones at debug. This
replaces the `run-safe!` helper, whose four copies had drifted, and
applies the guarantee to every emit site instead of only storage.
The metrics instance precondition is a plain assert, and the collector
lookup stays outside the recording guard, so a missing instance fails
hard even when asserts are disabled. Metrics is therefore no longer
optional: the storage, s3-backend and db-pool schemas require
`::mtx/metrics`, and the assets handler cfg always carries it.
`wrap-publisher` no longer returns nil for a nil instance, and the db
pool wires the prometheus tracker unconditionally.
AI-assisted-by: deepseek-v4.1-flash
* ⬆️ Update pnpm and its deps
* ⬆️ Update JVM dependencies in backend and common
Update several JVM dependencies across backend and common:
- passay 1.6.6 -> 2.0.0 (package reorg, ctor-based rules)
- siphash 2.0.0 -> 3.0.0 (SipHasher* renamed to SipHash*)
- lettuce-core, guava, sqlite-jdbc, jsoup, lz4-java, markdown-clj,
awssdk s3/sts, selmer, jackson-core/databind, shadow-cljs
Adapt passay validation to the new API (moved packages, constructor
configuration) and siphash to the renamed classes. Add tests for
password validation and UUID advisory-lock hashing.
AI-assisted-by: deepseek-v4-flash
* ⬆️ Update node on docker images
* 📎 Minor fixes related to pnpm12 compatibility
* ✨ Add dedicated concurrency limit for restore-file-snapshot
This adds a dedicated climit configuration for the restore-file-snapshot
RPC method with :permits 1 per profile (plus queue of 2 and 60s timeout)
and a global limit of 3. Previously the method only used the generic
root/by-profile and root/global limits, allowing up to 7 concurrent
restore operations per profile which caused database row lock contention
on FOR UPDATE and connection pool exhaustion.
* ✨ Skip locking on restore! to avoid blocking other operations
Changes the row lock acquisition in restore! from a blocking FOR UPDATE
to FOR UPDATE SKIP LOCKED. If the file row is already locked by another
concurrent operation (e.g., another restore or an update-file), the query
returns no rows and the caller fails fast with a clear conflict error
instead of blocking indefinitely holding a database connection.
* ✨ Add queue and timeout limits to root/by-profile concurrency limit
Previously root/by-profile had no queue limit (unbounded Integer/MAX_VALUE)
and no timeout, allowing requests to pile up indefinitely behind a profile
whose permits were exhausted by long-running operations. This could lead
to memory pressure and cascading failures. Now limited to 30 queued
requests with a 30-second timeout so excess requests fail fast.
* ✨ Move backup snapshot creation outside restore transaction
The backup snapshot (fsnap/create!) is now created in its own short-lived
connection before the actual restore transaction begins. This ensures the
backup is persisted independently of the restore outcome and reduces the
restore transaction window.
The restore itself runs inside a db/tx-run! block with an optimistic
locking check: it reads the file with FOR UPDATE and compares its revn
against the value captured at backup time. If the file was edited
concurrently, the restore aborts with a conflict error to prevent data
loss.
Co-dependent with the SKIP LOCKED change in restore! — the FOR UPDATE
acquired here is in the same transaction as restore!, so the SKIP LOCKED
inside restore! correctly sees the row as unlocked (same transaction).
* ♻️ Remove unused private function get-minimal-file
The local get-minimal-file function in file_snapshots.clj is no longer
used since restore! switched to direct exec-one! with FOR UPDATE SKIP
LOCKED. The sql:get-minimal-file SQL constant is still used directly.
* ✨ Add minor improvements on db connection management
* ♻️ Refactor create-file-snapshot to use explicit transaction management
Remove automatic transaction wrapping (`::db/transaction true`) and
pass `cfg` through the call chain instead of destructured `conn`.
Wrap `fsnap/create!` in an explicit `db/tx-run!` for clearer
transaction boundaries.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
* ✨ Add dedicated concurrency limit for create-file-snapshot
This adds a dedicated climit configuration for the create-file-snapshot
RPC method with :permits 1 per profile (plus queue of 2 and 60s timeout)
and a global limit of 3. Previously the method only used the generic
root/by-profile and root/global limits, allowing up to 10 concurrent
snapshot creation operations per profile which could cause database
contention and connection pool exhaustion.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
---------
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
