feat(server): add blob-size histogram and oversized-blob warn log - #240
Merged
Merged
Conversation
Inline binary_payload size drives memory pressure on both the worker (WAL merge buffers, resident manifests) and the master (compaction rewrite buffer), but there was no way to see it: the only blob metric was rollout_blob_budget_rejections_total, which fires solely when ROLLOUT_MAX_INFLIGHT_BLOB_BYTES is set (default 0 = disabled). Capacity problems were invisible until a pod died. Emit a rollout_blob_bytes histogram per inline blob on the add path, and warn when a single blob crosses ROLLOUT_LARGE_BLOB_LOG_BYTES (default 16 MiB, 0 disables) with the experiment and record id attached. Instrumentation is placed where the multipart and JSON parse paths converge rather than inside parse_multipart_rollouts' blob loop as the issue suggested: a JSON body populates binary_payload directly and never enters that loop, so instrumenting there would silently miss inline-JSON uploads. The converged site also already has the experiment name in scope, so no threading through the parser is needed. The histogram is intentionally unlabelled. Tagging by experiment is the more useful signal but each label value is its own time series, and a deployment can carry thousands of experiments; the warn log supplies the identifying detail for the outliers that actually matter. Tests, both verified to fail when the behavior is reverted: - blob_sizes_are_recorded_for_records_with_payloads asserts via a DebuggingRecorder that exactly the payload-carrying records produce samples (a payload-less record contributes nothing), covering the inline-JSON shape the multipart loop would have missed. - large_blob_log_threshold_is_inclusive_and_zero_disables pins the boundary and the disable switch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Motivation
Inline
binary_payloadsize drives memory pressure on both the worker (WAL merge buffers, resident manifests) and the master (compaction rewrite buffer) — but there was no way to see it. The only blob metric wasrollout_blob_budget_rejections_total, which fires solely whenROLLOUT_MAX_INFLIGHT_BLOB_BYTESis set (default0= disabled). Capacity problems stayed invisible until a pod died.This is the missing observability behind #229 (compaction memory) and #238 (worker WAL-merge memory): both bounded memory as a function of blob-carrying rows, but neither surfaced how big those blobs actually are.
Changes
rollout_blob_byteshistogram — recorded per inline blob on the add path.ROLLOUT_LARGE_BLOB_LOG_BYTES(new, default 16 MiB,0disables), logged withexperimentandrecord_id.One deviation from the issue
The issue suggests instrumenting inside
parse_multipart_rollouts' blob loop, with a parenthetical to "also cover the inline-JSON path."I placed the instrumentation where the multipart and JSON parse paths converge instead, because a JSON body populates
binary_payloaddirectly and never enters that loop — instrumenting there would silently miss inline-JSON uploads entirely. The converged site also already has the experiment name in scope, so nothing needs threading through the parser.Cardinality
The histogram is deliberately unlabelled, following the issue's own nice-to-have reasoning: a deployment can carry thousands of experiments and each label value is its own time series. The warn log supplies the identifying detail for the outliers that actually matter.
Tests
Both verified to fail when the behavior is reverted:
blob_sizes_are_recorded_for_records_with_payloads— uses aDebuggingRecorderto assert the emitted samples are exactly[512, 2048], proving a payload-less record contributes nothing. It exercises the inline-JSON record shape, i.e. precisely what the issue's suggested location would have missed. (Deleting thehistogram!call → fails.)large_blob_log_threshold_is_inclusive_and_zero_disables— pins the inclusive boundary and the0disable switch. (Changing>=to>→ fails.)Full workspace suite green: server 68 → 70, core 222,
fmt+clippy -D warningsclean.metrics-utilis added as a dev-dependency, matching the existing usage inlance-context-core.🤖 Generated with Claude Code