Repository navigation
Align queue concurrency override handling with River Pro - #704
Merged
Merged
Conversation
The queue update endpoint stores whatever `global_limit` and `local_limit` it receives in `river_queue.metadata.concurrency`. The form's `min="0"` attribute is the only guard, so a direct API call (or a crafted request) can persist a negative limit that River Pro has no meaningful interpretation for. Validate the concurrency override before opening a transaction and return a 400 via `apierror.NewBadRequestf` when either limit is negative, matching the endpoint's other bad request responses. Zero remains valid because it means "no limit". Rejecting the request up front keeps invalid configuration out of the database entirely rather than relying on every reader to cope with it. Add queue update tests covering a negative global limit and a negative local limit.
River Pro treats a missing `concurrency` key, `null`, and `{}` in
`river_queue.metadata` as "no override", and any non-empty object as a
full override. The queue serializer unmarshals the key into a
`*ConcurrencyConfig`, which yields nil for a missing key or `null` but a
non-nil zero config for `{}`. The queue detail page then shows the
override toggle as enabled with 0/0 limits even though River Pro is
using each client's own configuration.
Extract the override through `queueConcurrencyFromMetadata`, which
decodes the raw `concurrency` value into a map first and returns nil
when it is absent, `null`, empty, or not an object. Only a non-empty
object decodes into a `ConcurrencyConfig`, with omitted fields left at
zero rather than merged, mirroring River Pro's semantics. The frontend
already treats a `null` concurrency as "no override", so no UI change is
needed.
Add unit tests for empty metadata, a missing key, `null`, `{}`,
malformed values, a full override, and a partial override, plus a queue
get endpoint test showing `{"concurrency": {}}` is returned as no
override.
The producer list decodes `metadata.concurrency` into a non-pointer
`ConcurrencyConfig` and always returns its address when the metadata
parses. A producer whose metadata has no `concurrency` key, `null`, or
`{}` is therefore reported with a zero config instead of none. The
queue detail page uses producer configs to prefill the concurrency
form and to decide whether clients agree, so these phantom zero
configs show up as 0/0 limits and can make clients look inconsistent.
Extract the config through `producerConcurrencyFromMetadata`, which
returns nil unless the `concurrency` object contains at least one of
`global_limit`, `local_limit`, or `partition`. Producers also keep
limiter state like `running` in that object, so it is ignored when
deciding whether a config is present. The API type and the frontend's
`ConcurrencyConfig | null` producer type already allow a missing
config, and the comparison logic handles `null`, so no frontend change
is needed.
Add unit tests for empty metadata, a missing key, `null`, `{}`,
running state without config, malformed values, explicit zero limits,
and a full config.
The queue update endpoint distinguishes an omitted `concurrency` field,
an explicit `null`, and an object through `ExplicitNullable`, but only
the object case has coverage, and none of the tests decode the request
from JSON or check what lands in `river_queue.metadata`. A regression
in how `null` decodes or in what the clear path writes would go
unnoticed.
Add queue update tests that decode real JSON request bodies. Clearing
with `{"concurrency": null}` replaces an existing override with `{}`
metadata and returns no override, which River Pro reads as "no
override". Setting a full object stores it verbatim, including a zero
`local_limit` and sorted `partition.by_args`, and returns the same
config.
bgentry
force-pushed
the
bg/concurrency-override-cleanup
branch
from
October 12, 2026 03:17
0458652 to
2cbda6b
Compare
brandur
approved these changes
Oct 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
River Pro is standardizing how it reads queue concurrency overrides in
river_queue.metadata.concurrency: a missing key,null, and{}all mean "no override", and any non-empty object is a full override where omitted fields are zero (no limit) and nothing is merged field by field. River UI is the main writer of these overrides, so this brings its reads and writes in line with those semantics.The queue update endpoint now rejects negative
global_limitorlocal_limitvalues with a 400 instead of storing them. Before, the form'smin="0"was the only guard. When reading queues,{"concurrency": {}}is now treated as no override instead of an enabled 0/0 override. The producer list no longer reports a zero config for producers whose metadata has no concurrency config. Limiter state such asrunningdoesn't count as config on its own.The clear path still writes
{}, and the frontend is unchanged because it already treats anullconcurrency as no override. New tests cover the validation, every read case, and clearing via{"concurrency": null}and setting a full override from real JSON request bodies, including what ends up in the queue's metadata.