Repository navigation
Throttle output webhooks and skip building throttled payloads - #3224
Open
itsrayeedscodespace wants to merge 1 commit into
Open
itsrayeedscodespace wants to merge 1 commit into
itsrayeedscodespace wants to merge 1 commit into
Conversation
Coglet sent an `output` webhook for every yielded chunk: should_send let Output events skip the throttle. Each payload is a full snapshot with all logs and outputs so far, so a model that logs 1 MB and yields 1,000 chunks sent about 1,000 webhooks and 1 GB. The snapshot was also built before the filter and throttle check, so events that were then dropped still cloned all logs and outputs. `output` events now share the throttle with `logs`, as docs/http.md and COG_THROTTLE_RESPONSE_INTERVAL describe and as the Python and Go runtimes did. Prediction checks should_send before building the payload. A dropped event loses no data: the next webhook, or the terminal one, has the full state. Fixes replicate#3219 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
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.
Fixes #3219.
Root cause
WebhookSender::should_sendreturnedtruefor everyOutputevent before the throttle check. Its comment said output was effectively unthrottled in the old Python runtime. It wasn't:ResponseThrottlerthrottled every non-terminal event, and the Go runtime'sSendConditionalthrottledlogsandoutputat 500ms.docs/http.mdandCOG_THROTTLE_RESPONSE_INTERVALindocs/environment.mdalso promise thatoutputandlogswebhooks go out at most once per interval.Every webhook is a full snapshot, with all logs and all outputs so far. One webhook per chunk therefore means O(chunks × logs) bytes.
Prediction::fire_webhookalso built that snapshot beforeshould_sendran, so throttled and filtered events still cloned all logs and outputs.Change
should_sendno longer exemptsOutput.outputandlogsshare the throttle again.fire_webhookchecksshould_sendbefore building the payload.should_sendis nowpub(crate);sendkeeps its signature and still checks.The terminal path is unchanged. Terminal webhooks are never throttled and always carry the full state, so a dropped progress webhook loses no data.
Before / after
Rust harness. This drives a real
PredictionandWebhookSenderagainst a local axum receiver that records count, bytes, peak in-flight requests and arrival order. 1,000 chunks 5ms apart, 500ms throttle, debug build. The harness wasn't committed.start,completed: time spent inPredictioncallsPredictioncalls (snapshot + serialization)In every run, the terminal webhook arrived last and carried all outputs and the full logs.
Issue repro in Docker. I ran the issue's
predict.pyand sink with locally built coglet wheels, linux/amd64 under emulation on Apple Silicon. I checked the_impl.abi3.sohash in each image against the wheel I built.log_bytesCOG_THROTTLE_RESPONSE_INTERVAL=5Under emulation the chunks reach coglet in one burst at the end of the prediction, on main as well. So this setup shows the count reduction but not the 500ms spacing; the Rust harness covers the spacing.
Relationship to #3211
#3211 sends progress webhooks one at a time from a single task per prediction and replaces a waiting request with a newer one. That bounds in-flight requests and fixes ordering. It doesn't throttle: a fast receiver drains the queue as fast as chunks arrive. The payload is also still built and serialized for every chunk. The harness on #3211 alone gives 1,002 webhooks and 1.0 GB with a fast receiver, and 19.8s of payload building.
The two changes are complementary. I applied this patch on top of #3211 and all 241 Rust tests passed. The conflicts are only in tests:
send_output_not_throttled; this PR renames it tosend_output_throttled.prediction.rs.pred.mark_streaming().With both applied, peak in-flight stays at 1 even when the receiver hangs for 5s.
Tests
prediction::tests::output_webhooks_are_throttled_and_terminal_has_everything: 100 KB of logs, 100 chunks, then succeeded, failed or canceled. Checks that only the start and terminal webhooks are sent, and that the terminal one has all 100 outputs and the full logs. On main it gets 102 webhooks.webhook::tests::send_output_throttledreplacessend_output_not_throttled.webhook::tests::should_send_throttles_output_and_logs_togetherandshould_send_respects_filter.All three new behavior tests fail with the
Outputexemption put back. I ran the new tests 30 times with no failures.Behavior changes and limits
outputevent within the throttle interval of the last webhook isn't sent. That includes thestartwebhook, so the firstoutputwebhook can be up to 500ms later than on main. The Python and Go runtimes behaved the same way.logstoday. If a model yields and then pauses, the latest outputs arrive with the next webhook or the terminal one. A trailing edge fits naturally in Send single outputs only in the final webhook, and send webhooks in order #3211's queue.logs, so each one is as large as the logs. The issue's suggestion to cap logs in non-terminal payloads would change the payload contract, so I left it for maintainers to decide.Verified and Tested with Claude Code