Repository navigation
perf(world-vercel): batch a fan-out's step-execution queue publishes #3838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
68b61ac
ee31c81
480b34f
367e5e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@workflow/world-vercel': patch | ||
| '@workflow/world': patch | ||
| '@workflow/core': patch | ||
| --- | ||
|
|
||
| Publish a fan-out's step-execution messages in one batched queue request instead of one per step, via a new optional `Queue.queueBatch` implemented on `@vercel/queue`'s `experimental_sendBatch`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1216,6 +1216,88 @@ export async function queueMessage( | |
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Publishes several messages to one logical queue, using the World's batch | ||
| * send when it has one and falling back to concurrent single sends when it | ||
| * does not. | ||
| * | ||
| * Rejects if ANY message failed to publish, because every caller so far wants | ||
| * all-or-nothing: the recovery is to fail the delivery and let redelivery | ||
| * republish the whole set, deduped by the per-message `idempotencyKey`. That | ||
| * means a partial batch can leave some messages already out — which is | ||
| * exactly why the keys are required rather than advisory. | ||
| */ | ||
| export async function queueMessages( | ||
| world: World, | ||
| queueName: Parameters<typeof world.queue>[0], | ||
| messages: readonly { | ||
| message: Parameters<typeof world.queue>[1]; | ||
| opts?: Parameters<typeof world.queue>[2]; | ||
| }[] | ||
| ): Promise<void> { | ||
| if (messages.length === 0) return; | ||
| const batch = world.queueBatch?.bind(world); | ||
| if (!batch) { | ||
| await Promise.all( | ||
| messages.map((entry) => | ||
| queueMessage(world, queueName, entry.message, entry.opts) | ||
| ) | ||
| ); | ||
| return; | ||
| } | ||
| await trace( | ||
| 'queue.publish', | ||
| { | ||
| attributes: { | ||
| ...Attribute.MessagingSystem('vercel-queue'), | ||
| ...Attribute.MessagingDestinationName(queueName), | ||
| ...Attribute.MessagingOperationType('publish'), | ||
| ...Attribute.MessagingBatchMessageCount(messages.length), | ||
| ...Attribute.PeerService('vercel-queue'), | ||
| ...Attribute.RpcSystem('vercel-queue'), | ||
| ...Attribute.RpcService('vqs'), | ||
| ...Attribute.RpcMethod('publishBatch'), | ||
| }, | ||
| kind: await getSpanKind('PRODUCER'), | ||
| }, | ||
| async () => { | ||
| const results = await batch(queueName, messages); | ||
| // A World that answers with the wrong number of results has told us | ||
| // nothing about the messages it left out. Treated as a failure of the | ||
| // whole batch rather than read as success for the entries that ARE | ||
| // present: republishing under the same idempotency keys is safe, | ||
| // silently never dispatching a step is not (the run makes no progress | ||
| // and nothing surfaces an error). | ||
| if (results.length !== messages.length) { | ||
| throw Object.assign( | ||
| new Error( | ||
| `Queue batch for ${queueName} returned ${results.length} ` + | ||
| `result(s) for ${messages.length} message(s)` | ||
| ), | ||
| { retryable: true } | ||
| ); | ||
| } | ||
| const failures = results.filter((result) => result.error !== undefined); | ||
| if (failures.length === 0) return; | ||
| const retryable = failures.some( | ||
| (failure) => failure.error !== undefined && failure.retryable | ||
| ); | ||
| const error = new Error( | ||
| `Failed to publish ${failures.length} of ${messages.length} queue ` + | ||
| `message(s) to ${queueName}: ${failures[0]?.error}` | ||
| ); | ||
| // Carried on the error so a caller CAN tell a transient partial batch | ||
| // from a permanent rejection. Nothing reads it yet: today every caller | ||
| // rejects the delivery either way, so a permanently rejected entry | ||
| // still costs the full redelivery budget. Left in place because the | ||
| // information is only available here, and a fast-fail on | ||
| // `retryable: false` needs it. | ||
| Object.assign(error, { retryable }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AI Review: That has a measurable cost. With one permanently-rejected entry in a 64-branch fan-out, the delivery burns all 48 redeliveries and ~2,900 redundant republishes before giving up. The unbatched path does the same, so this isn't a regression — but the batched path is the one that now has the information and discards it, which makes a fast-fail on |
||
| throw error; | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Calculates the queue overhead time in milliseconds for a given message. | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AI Review:
queueMessagesonly inspectserror, neverresults.length. A World whosequeueBatchreturns a short array reports success here:handleSuspensionresolves, the delivery is acked, and those steps are never dispatched — the run wedges with no error anywhere in the system.world-vercelguards this internally (toBatchResult(undefined)) and the SDK length-checks too, so it's unreachable through the world in this PR. ButqueueBatchis documented inbuilding-a-world.mdxfor third-party worlds, which makes it an unenforced contract at exactly the boundary that publishes it — and unlike a rejection, this failure is silent.I reproduced it with a 64-branch fan-out against a world returning half the results: 32 of 63 steps silently lost,
handleSuspensionresolved without error. Adding the length check flips it to a clean rejection that converges on redelivery, and all 143 tests inhelpers.test.ts+suspension-handler.test.tsstill pass: