Summary
The worker still treats a pull of queue items as belonging to a single provider, which stopped being true in #74. Two consequences follow from that one assumption, and they are cheaper to fix together than apart, because most of what either needs is the same.
One provider's rate limit stalls every other provider. provider_cooldown_until is a single file-static deadline (src/worker.c:94), set by provider_begin_cooldown() after a 429 and checked at the top of process_queue_batch(), which returns immediately if it has not passed. So a hosted provider on a free tier holds up a local Ollama that has no rate limit at all. Latency rather than correctness: the items stay pending and are picked up when the cooldown expires, and #74 already stopped the deferral being charged to the wrong provider's items.
One provider that cannot be resolved stalls every other provider. A vectorizer naming a provider that does not exist, or one whose init() fails, raises:
provider = get_embedding_provider(providers[batch_start]);
if (provider == NULL)
elog(ERROR, "embedding provider \"%s\" is not available",
providers[batch_start]);The raise aborts the transaction, so the whole pull returns to pending, including items whose provider is perfectly fine. The next pull selects the same oldest items, reaches the same bad group and aborts again. The batch backoff keeps this from spinning, growing the wait to BATCH_RETRY_MAX, but nothing drains and nothing fails. Before #74 this could not discriminate, since a bad provider meant nothing could proceed anyway; now the failure of one table's configuration is borne by every other table.
Why one issue
Both need the same two things: per-provider state keyed on the provider name, in place of the single file-static, and the ability to skip one request group whilst the rest of the pull proceeds, returning its items to pending uncharged rather than aborting. #74 already put a seed of this in the rate-limit deferral loop, which compares providers[idx] against providers[batch_start] so that one provider's 429 does not charge another's items; both halves of this issue are about generalising that comparison into something the whole function understands.
Doing them separately means building a narrow version of that state twice.
Where they differ
Each half has one part the other does not give it for free.
The cooldown needs the claim filtered. The pull selects by age, so unless the SELECT itself excludes items whose resolved provider is cooling down, every poll claims them, marks them processing and puts them straight back: churn on the queue table for the length of the cooldown. The resolved provider is already available in that query, since inheritance resolves there, so this is a filter rather than new plumbing.
The resolution failure needs a backoff that is not an exception. Whether a provider name resolves is only knowable in C, after the rows are fetched, so it cannot be filtered the same way. The batch backoff is today reachable only by throwing: queue_item_record_failure() returning false is what tells the main loop to grow batch_retry_interval. Skipping a group without throwing means a pull in which every group is unresolvable would spin at the poll interval, so process_queue_batch() needs a way to report a batch-level fault back to its caller without an exception.
What must not change
Items must not be charged for a provider that cannot be resolved. test/t/005_batch_failure_backoff.pl exists to prevent exactly that and injects exactly this fault, describing it as the most likely way a real deployment lands here:
A failure belonging to the batch has none of that. Nothing is charged, deliberately, since billing a blameless item for a misconfigured provider would work through the queue retiring one innocent row per max_attempts cycles.
A single mistyped pgedge_vectorizer.provider would otherwise mark the whole queue failed, recoverable only with retry_failed() after fixing the setting. The item is blameless; the configuration is wrong. Any fix has to skip the group and leave its items uncharged and still reach the backoff when nothing in a pull could be attempted.
Notes
Raised whilst implementing #27 and during review of #77. Supersedes the separate rate-limit issue this one began as, and #78, which is closed in favour of it.
Summary
The worker still treats a pull of queue items as belonging to a single provider, which stopped being true in #74. Two consequences follow from that one assumption, and they are cheaper to fix together than apart, because most of what either needs is the same.
One provider's rate limit stalls every other provider.
provider_cooldown_untilis a single file-static deadline (src/worker.c:94), set byprovider_begin_cooldown()after a 429 and checked at the top ofprocess_queue_batch(), which returns immediately if it has not passed. So a hosted provider on a free tier holds up a local Ollama that has no rate limit at all. Latency rather than correctness: the items staypendingand are picked up when the cooldown expires, and #74 already stopped the deferral being charged to the wrong provider's items.One provider that cannot be resolved stalls every other provider. A vectorizer naming a provider that does not exist, or one whose
init()fails, raises:The raise aborts the transaction, so the whole pull returns to
pending, including items whose provider is perfectly fine. The next pull selects the same oldest items, reaches the same bad group and aborts again. The batch backoff keeps this from spinning, growing the wait toBATCH_RETRY_MAX, but nothing drains and nothing fails. Before #74 this could not discriminate, since a bad provider meant nothing could proceed anyway; now the failure of one table's configuration is borne by every other table.Why one issue
Both need the same two things: per-provider state keyed on the provider name, in place of the single file-static, and the ability to skip one request group whilst the rest of the pull proceeds, returning its items to
pendinguncharged rather than aborting. #74 already put a seed of this in the rate-limit deferral loop, which comparesproviders[idx]againstproviders[batch_start]so that one provider's 429 does not charge another's items; both halves of this issue are about generalising that comparison into something the whole function understands.Doing them separately means building a narrow version of that state twice.
Where they differ
Each half has one part the other does not give it for free.
The cooldown needs the claim filtered. The pull selects by age, so unless the SELECT itself excludes items whose resolved provider is cooling down, every poll claims them, marks them
processingand puts them straight back: churn on the queue table for the length of the cooldown. The resolved provider is already available in that query, since inheritance resolves there, so this is a filter rather than new plumbing.The resolution failure needs a backoff that is not an exception. Whether a provider name resolves is only knowable in C, after the rows are fetched, so it cannot be filtered the same way. The batch backoff is today reachable only by throwing:
queue_item_record_failure()returning false is what tells the main loop to growbatch_retry_interval. Skipping a group without throwing means a pull in which every group is unresolvable would spin at the poll interval, soprocess_queue_batch()needs a way to report a batch-level fault back to its caller without an exception.What must not change
Items must not be charged for a provider that cannot be resolved.
test/t/005_batch_failure_backoff.plexists to prevent exactly that and injects exactly this fault, describing it as the most likely way a real deployment lands here:A single mistyped
pgedge_vectorizer.providerwould otherwise mark the whole queuefailed, recoverable only withretry_failed()after fixing the setting. The item is blameless; the configuration is wrong. Any fix has to skip the group and leave its items uncharged and still reach the backoff when nothing in a pull could be attempted.Notes
Raised whilst implementing #27 and during review of #77. Supersedes the separate rate-limit issue this one began as, and #78, which is closed in favour of it.