From da31487a969967fe16f0994aab5fa2a3002702cb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 26 Jul 2026 13:32:32 +0000 Subject: [PATCH] fix(queue): drop version-queue special-case 30 retries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VERSION_QUEUE_MAX_READS=30 was a band-aid for timeout redeliveries on large deleted manifests. Cleanup already commits per file, and the sweeper requeues leftovers — keep the shared MAX_QUEUE_READS=5 budget for every queue. Co-authored-by: Martin DONADIEU --- .../_backend/triggers/on_version_update.ts | 2 ++ .../_backend/triggers/queue_consumer.ts | 8 ++++---- .../queue-consumer-message-shape.unit.test.ts | 18 +++++++++--------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/supabase/functions/_backend/triggers/on_version_update.ts b/supabase/functions/_backend/triggers/on_version_update.ts index 51ab82d29b..419e211af6 100644 --- a/supabase/functions/_backend/triggers/on_version_update.ts +++ b/supabase/functions/_backend/triggers/on_version_update.ts @@ -278,6 +278,8 @@ type ManifestCleanupEntry = { /** * Trash unreferenced R2 objects first (exist → move to deleted-after-7-days/, * missing → ok), then delete that DB row. Never drop DB tracking before R2 is handled. + * Per-file work is committed, so a timeout mid-pass is safe to retry. Leftover rows + * after the normal retry budget are reclaimed by sweep_deleted_version_manifests. * Incomplete work throws so the queue retries; already-trashed paths are idempotent. */ async function deleteManifest(c: Context, record: Database['public']['Tables']['app_versions']['Row']) { diff --git a/supabase/functions/_backend/triggers/queue_consumer.ts b/supabase/functions/_backend/triggers/queue_consumer.ts index 67212720f6..45e660ad50 100644 --- a/supabase/functions/_backend/triggers/queue_consumer.ts +++ b/supabase/functions/_backend/triggers/queue_consumer.ts @@ -26,7 +26,6 @@ const QUEUE_HTTP_TIMEOUT_MS = 15_000 const VERSION_QUEUE_HTTP_TIMEOUT_MS = 300_000 // large deleted manifests: trash then DB delete const HEALTHCHECK_HTTP_TIMEOUT_MS = 8_000 export const MAX_QUEUE_READS = 5 -const VERSION_QUEUE_MAX_READS = 30 // deleted manifests can need many partial trash/delete passes const DISCORD_IGNORED_ERROR_CODES = new Set(['version_not_found', 'no_channel']) export const messageSchema = z.object({ @@ -281,9 +280,10 @@ function getQueueHttpTimeoutMs(functionName: string): number { return QUEUE_HTTP_TIMEOUT_MS } -function getQueueMaxReads(queueName: string): number { - if (isVersionQueueFunction(queueName)) - return VERSION_QUEUE_MAX_READS +function getQueueMaxReads(_queueName: string): number { + // Same budget for every queue. Large deleted-manifest cleanup already commits + // per file; leftover rows are reclaimed by sweep_deleted_version_manifests. + // Do not burn retries as a fake "progress" budget. return MAX_QUEUE_READS } diff --git a/tests/queue-consumer-message-shape.unit.test.ts b/tests/queue-consumer-message-shape.unit.test.ts index 605e269789..624cd7f826 100644 --- a/tests/queue-consumer-message-shape.unit.test.ts +++ b/tests/queue-consumer-message-shape.unit.test.ts @@ -159,8 +159,8 @@ describe('queue_consumer legacy message compatibility', () => { expect(__queueConsumerTestUtils__.getQueueVisibilityTimeout('cron_email')).toBe(120) expect(__queueConsumerTestUtils__.getQueueVisibilityTimeout('on_version_update')).toBe(900) expect(__queueConsumerTestUtils__.getQueueHttpTimeoutMs('on_version_update')).toBe(300_000) - expect(__queueConsumerTestUtils__.getQueueMaxReads('on_version_update')).toBe(30) - expect(__queueConsumerTestUtils__.getQueueMaxReads('on_manifest_create')).toBe(5) + expect(__queueConsumerTestUtils__.getQueueMaxReads('on_version_update')).toBe(MAX_QUEUE_READS) + expect(__queueConsumerTestUtils__.getQueueMaxReads('on_manifest_create')).toBe(MAX_QUEUE_READS) expect(__queueConsumerTestUtils__.getQueueHttpTimeoutMs('cron_email')).toBe(15_000) expect(__queueConsumerTestUtils__.shouldRunQueueSyncInBackground('on_manifest_create')).toBe(false) expect(__queueConsumerTestUtils__.shouldRunQueueSyncInBackground('cron_email')).toBe(true) @@ -237,8 +237,8 @@ describe('queue_consumer legacy message compatibility', () => { )).toBe('continue') }) - it.concurrent('uses the version queue retry budget for Discord failure alerts', () => { - const versionRetryBudget = __queueConsumerTestUtils__.getQueueMaxReads('on_version_update') + it.concurrent('uses the shared queue retry budget for Discord failure alerts', () => { + const retryBudget = __queueConsumerTestUtils__.getQueueMaxReads('on_version_update') const midRetry = { cf_id: 'cf-version-mid', error_code: 'manifest_cleanup_incomplete', @@ -246,7 +246,7 @@ describe('queue_consumer legacy message compatibility', () => { function_type: 'supabase', msg_id: 2, payload_size: 10, - read_count: MAX_QUEUE_READS, + read_count: MAX_QUEUE_READS - 1, status: 500, status_text: 'Internal Server Error', } @@ -254,12 +254,12 @@ describe('queue_consumer legacy message compatibility', () => { ...midRetry, cf_id: 'cf-version-done', msg_id: 3, - read_count: versionRetryBudget, + read_count: retryBudget, } - expect(versionRetryBudget).toBe(30) - expect(__queueConsumerTestUtils__.getActionableQueueFailures([midRetry], versionRetryBudget)).toEqual([]) - expect(__queueConsumerTestUtils__.getActionableQueueFailures([exhausted], versionRetryBudget)).toEqual([exhausted]) + expect(retryBudget).toBe(MAX_QUEUE_READS) + expect(__queueConsumerTestUtils__.getActionableQueueFailures([midRetry], retryBudget)).toEqual([]) + expect(__queueConsumerTestUtils__.getActionableQueueFailures([exhausted], retryBudget)).toEqual([exhausted]) }) it.concurrent('alerts Discord after retry budget is exhausted', () => {