* ♻️ Take the command limits before the root ones in the RPC limiter
The RPC concurrency limiter took the root limiters (the ones every request
passes through) before the ones a command declares, so a request waiting for
the limits of its own command held the root permits of its profile while it
waited: a profile with a pile-up of heavy requests saw its other requests
queue behind them and be rejected with 429.
That order was forced by two commands that entered the limiter from inside
their handler: a command that declares no limits of its own can only take
them deeper than the root ones, so two paths of one profile could take the
same limiters in opposite order and park for good.
The commands declare their limits in their metadata now and their handlers
call the work directly, so no command enters the limiter from inside its
handler and every path takes the limiters in the same order. The root
limiters are appended to the sequence first, so the limits a command
declares end up outermost and are taken first: a request waiting for them
does not hold the root permits of its profile. The photo of a team keeps its
limits, because it declares them now that the helper does not.
A call from inside a handler can only reenter the limits that thread
already holds, and it is refused when it asks for a new one, because that
would take it after the root ones and invert the order against the other
paths. The key of a limit is resolved the same way in every path, so both
paths point at the same limiter and the reentrancy check sees it.
Every entry of the shipped climit.edn declares a queue, so the number of
requests waiting for the same limit is bounded instead of piling up until
the timeout. The production file lives outside the repository and has to be
mirrored to pick them up.
The tests cover the acquisition order of a command with and without limits
of its own, the order of the limits as a whole (a ring of any size fails,
not only a pair in opposite order), the nested call that asks for a limit
the thread does not hold, and the queues and timeouts of the shipped
configuration. The import of a binfile declares its request as multipart in
its test, the way the commands that take an upload map do.
AI-assisted-by: minimax-m3.1-flash
AI-assisted-by: deepseek-v4.1-flash
* 🔧 Keep the commands' limits and the shipped config in sync
`profile-plugin-ops` guarded only an UPDATE on the caller's own profile row,
which the row lock already serializes, so its global cap could only reject a
burst of different profiles toggling plugins. Both commands declare the
by-profile limit alone now, and the example config drops the entry.
A declared id without an entry does not limit anything: `wrap` logs a warning
and runs the handler with no limiter at all, which is how a command ends up
unbounded in production. A test fails now when a command declares an id the
config does not list, and the header of the example says the production file
mirrors the shape and lists every declared id, not the values.
AI-assisted-by: deepseek-v4.1-flash
* 📚 Record that background work is not waited for with a sleep
A long command moved to the background reports its own completion, so
polling it with a sleep inside a foreground command only blocks the session
and the notification arrives anyway. The rule says to keep working on
something else, or to end the response and wait for it.
AI-assisted-by: minimax-m3.1-flash
* ✨ Extend backend histogram buckets beyond 7.5s
Give every backend timing histogram a tail past the shared 7500ms cap
so slow RPC calls, CLIMIT waits and background tasks report real
quantiles instead of clipping at the top bucket.
Request and wait metrics grow to 60s, background tasks to 300s and
Redis eval to 10s; the sub-7.5s buckets stay untouched to keep the
history comparable.
Closes#12408
AI-assisted-by: muse-spark-1.3-contributor
* 🐛 Fix flaky assertion in climit republish test
Read the last published waiting value instead of the exact
publication list. The sampler thread started with the component
runs its first tick asynchronously, and when it lands inside the
recording window it publishes the same count twice, which made
the full-suite CI run fail while focused runs stayed green.
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Add file_data blob to JSON decode helper
Allow decoding file_data blobs (app.util.blob) to JSON
for investigations with jq, offline from a blob file or
from nREPL with the backend running.
AI-assisted-by: muse-spark-1.3-contributor
* ✨ Add validation step timings to update-file logs and metrics
Measure the six phases inside process-changes-and-validate
(resolve-libs, process-changes, soft and hard affected and
schema validations) with tpoint and try/finally, so timings
are recorded even when a hard validation throws.
Publish each phase to a new histogram
penpot_rpc_update_file_validation_timing with a fixed step
label using the request buckets (up to 60000 ms), and extend
the update-file debug log with the same *-ms fields plus
file, project and team ids. The deferred log only runs on
success, so failures get an inline error log with partial
timings.
AI-assisted-by: muse-spark-1.3-contributor
* 💄 Translate decode-file-data comments to English
Translate the Spanish docstrings, section headers and test
messages of the file_data decode helper to English, keeping
code untouched. The repo convention is English in code and
comments; the helper was first written in Spanish.
AI-assisted-by: muse-spark-1.3-contributor
The merge of main into staging brought the closed storage metadata
schema, which requires file-id and id on file-data objects. The
restore test built one without those keys, so it failed with an
invalid metadata error. Its helper now takes extra metadata and
the test passes the ids of the file and of its file-data row, as
production does.
The republish test asserted the exact list of waiting-gauge
publications, but the component's sampler starts its first tick right
away and under load lands inside the recorded window, publishing the
same count twice. The test now builds the counters alone, so the
republish call is the only publisher.
AI-assisted-by: deepseek-v4.1-flash
The queue gauge was written by whichever limiter of a name ran last, so a
per-profile pile-up never showed: a healthy profile writing 0 overwrote the
15 of the profile that was queueing (4 of 1441 samples in production), and
a limiter whose requests never come back read 0 while 72 threads stayed
parked.
A queue is an entry and an exit, so the new gauge is accumulated: one
counter per limiter name and instance, up when a call starts waiting for a
permit, down when it stops, so a call that never leaves stays visible. It
is always written with `:val`, and a daemon sampler republishes every count
every 15s so a name whose traffic died does not go stale in the scrape;
halting the component stops that thread.
Rejections had no metric at all: penpot_rpc_climit_rejected_total counts
them by name and reason (queue-full or timeout), and the log lines now
carry queue-max, permits, code and reason, so an operator can tell whether
the limiter did its job or was saturated.
The gauges that could not tell the truth are gone: queue and permits (same
last-write-wins problem, and waiting plus the wait histogram replace them)
and http_worker_busy_threads (reads 0 on an instance with 72 blocked
threads; pool_size is the signal). No other dashboard or alert rule reads
them.
AI-assisted-by: deepseek-v4.1-flash
An instance left the audit events pipeline dead: every push-audit-events
answered with 429 and 69 worker threads parked for good inside a limiter.
Two code paths of one profile acquired the same two limiters in opposite
order: a command declaring its own limits (upload-file-media-object) took
process-image/by-profile and then root/by-profile, while a command with no
limits of its own took the roots first and then re-entered climit from
inside its handler with the same ids (create-file-media-object-from-url).
Each path held what the other one waited for, and no limiter had a timeout
to break the wait.
Now wrap puts the root limiters after the command's own, so they end up
outermost and every path meets them first: the nested chains carry only the
command's own limits and run inside them, which puts both paths in the same
order. impl-invoke also keeps, per thread, the set of limiters that thread
is inside of and runs the handler inline when a call asks for one it
already holds, so a handler can no longer queue behind itself.
The shipped climit.edn sets a timeout on every limiter (60s for operations
that can be slow, 30s otherwise), so no wait can park a thread for ever.
The production file lives outside the repository and has to be mirrored.
Tests cover the acquisition order, the deadlock, the reentrancy and the
timeouts of the shipped config.
AI-assisted-by: deepseek-v4.1-flash
